public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org, yingwen.chen@intel.com,
	devel@edk2.groups.io, phillip.goerl@oracle.com,
	alex.williamson@redhat.com, jiewen.yao@intel.com,
	jun.nakajima@intel.com, michael.d.kinney@intel.com,
	pbonzini@redhat.com, boris.ostrovsky@oracle.com,
	rfc@edk2.groups.io, joao.m.martins@oracle.com
Subject: Re: [PATCH] q35: lpc: allow to lock down 128K RAM at default SMBASE address
Date: Wed, 11 Sep 2019 19:30:46 +0200	[thread overview]
Message-ID: <8ed1ab55-d5ee-9703-dd28-cf50fbda5408@redhat.com> (raw)
In-Reply-To: <20190910175841.176b26e4@redhat.com>

On 09/10/19 17:58, Igor Mammedov wrote:
> On Mon, 9 Sep 2019 21:15:44 +0200
> Laszlo Ersek <lersek@redhat.com> wrote:

[...]

> It looks like fwcfg smi feature negotiation isn't reusable in this case.
> I'd prefer not to add another device just for another SMI feature
> negotiation/activation.

I thought it could be a register on the new CPU hotplug controller that
we're going to need anyway (if I understand correctly, at least).

But:

> How about stealing reserved register from pci-host similar to your
> extended TSEG commit (2f295167 q35/mch: implement extended TSEG sizes)?
> (Looking into spec can (ab)use 0x58 or 0x59 register)

Yes, that should work.

In fact, I had considered 0x58 / 0x59 when looking for unused registers
for extended TSEG configuration:

http://mid.mail-archive.com/d8802612-0b11-776f-b209-53bbdaf67515@redhat.com

So I'm OK with this, thank you.

More below:

>> ... I've done some testing too. Applying the QEMU patch on top of
>> 89ea03a7dc83, my plan was:
>>
>> - do not change OVMF, just see if it continues booting with the QEMU
>> patch
>>
>> - then negotiate bit#1 too, in step (1a) -- this is when I'd expect (3a)
>> to break.
>>
>> Unfortunately, the result is worse than that; even without negotiating
>> bit#1 (i.e. in the baseline test), the firmware crashes (reboots) in
>> step (3a). I've checked "info mtree", and all occurences of
>> "smbase-blackhole" and "smbase-blackhole" are marked [disabled]. I'm not
>> sure what's wrong with the baseline test (i.e. without negotiating
>> bit#1). If I drop the patch (build QEMU at 89ea03a7dc83), then things
>> work fine.
> 
> that was a bug in my code, which always made lock down effective on
> feature_ok selection, which breaks relocation for reasons you've
> explained above.
> 
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index 17a8cd1b51..32ddf54fc2 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -383,7 +383,7 @@ static const MemoryRegionOps smbase_blackhole_ops = {
>  
>  static void ich9_lpc_smbase_locked_update(ICH9LPCState *lpc)
>  {
> -    bool en = lpc->smi_negotiated_features & ICH9_LPC_SMI_F_LOCKED_SMBASE_BIT;
> +    bool en = lpc->smi_negotiated_features & (UINT64_C(1) << ICH9_LPC_SMI_F_LOCKED_SMBASE_BIT);
>  
>      memory_region_transaction_begin();
>      memory_region_set_enabled(&lpc->smbase_blackhole, en);

I see.

ICH9_LPC_SMI_F_LOCKED_SMBASE_BIT is 1, with the intended value for
bitmask checkin) being 1LLU<<1 == 2LLU.

Due to the bug, the function would check value 1 in the bitmask -- which
in fact corresponds to bit#0. Bit#0 happens to be
ICH9_LPC_SMI_F_BROADCAST_BIT.

And because OVMF would negotiate that feature (= broadcast SMI) even in
the baseline test, it ended up enabling the "locked smbase" feature too.

So ultimately I think we can consider this a valid test (= with
meaningful result); the result is that we can't reuse these fw_cfg files
for "locked smbase", as discussed above.

Thanks!
Laszlo

  reply	other threads:[~2019-09-11 17:31 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-13 14:16 CPU hotplug using SMM with QEMU+OVMF Laszlo Ersek
2019-08-13 16:09 ` Laszlo Ersek
2019-08-13 16:18   ` Laszlo Ersek
2019-08-14 13:20   ` Yao, Jiewen
2019-08-14 14:04     ` Paolo Bonzini
2019-08-15  9:55       ` Yao, Jiewen
2019-08-15 16:04         ` Paolo Bonzini
2019-08-15 15:00       ` [edk2-devel] " Laszlo Ersek
2019-08-15 16:16         ` Igor Mammedov
2019-08-15 16:21         ` Paolo Bonzini
2019-08-16  2:46           ` Yao, Jiewen
2019-08-16  7:20             ` Paolo Bonzini
2019-08-16  7:49               ` Yao, Jiewen
2019-08-16 20:15                 ` Laszlo Ersek
2019-08-16 22:19                   ` Alex Williamson
2019-08-17  0:20                     ` Yao, Jiewen
2019-08-18 19:50                       ` Paolo Bonzini
2019-08-18 23:00                         ` Yao, Jiewen
2019-08-19 14:10                           ` Paolo Bonzini
2019-08-21 12:07                             ` Laszlo Ersek
2019-08-21 15:48                           ` [edk2-rfc] " Michael D Kinney
2019-08-21 17:05                             ` Paolo Bonzini
2019-08-21 17:25                               ` Michael D Kinney
2019-08-21 17:39                                 ` Paolo Bonzini
2019-08-21 20:17                                   ` Michael D Kinney
2019-08-22  6:18                                     ` Paolo Bonzini
2019-08-22 18:29                                       ` Laszlo Ersek
2019-08-22 18:51                                         ` Paolo Bonzini
2019-08-23 14:53                                           ` Laszlo Ersek
2019-08-22 20:13                                         ` Michael D Kinney
2019-08-22 17:59                               ` Laszlo Ersek
2019-08-22 18:43                                 ` Paolo Bonzini
2019-08-22 20:06                                   ` Michael D Kinney
2019-08-22 22:18                                     ` Paolo Bonzini
2019-08-22 22:32                                       ` Michael D Kinney
2019-08-22 23:11                                         ` Paolo Bonzini
2019-08-23  1:02                                           ` Michael D Kinney
2019-08-23  5:00                                             ` Yao, Jiewen
2019-08-23 15:25                                               ` Michael D Kinney
2019-08-24  1:48                                                 ` Yao, Jiewen
2019-08-27 18:31                                                   ` Igor Mammedov
2019-08-29 17:01                                                     ` Laszlo Ersek
2019-08-30 14:48                                                       ` Igor Mammedov
2019-08-30 18:46                                                         ` Laszlo Ersek
2019-09-02  8:45                                                           ` Igor Mammedov
2019-09-02 19:09                                                             ` Laszlo Ersek
2019-09-03 14:53                                                               ` [Qemu-devel] " Igor Mammedov
2019-09-03 17:20                                                                 ` Laszlo Ersek
2019-09-04  9:52                                                                   ` imammedo
2019-09-05 13:08                                                                     ` Laszlo Ersek
2019-09-05 15:45                                                                       ` Igor Mammedov
2019-09-05 15:49                                                                       ` [PATCH] q35: lpc: allow to lock down 128K RAM at default SMBASE address Igor Mammedov
2019-09-09 19:15                                                                         ` Laszlo Ersek
2019-09-09 19:20                                                                           ` Laszlo Ersek
2019-09-10 15:58                                                                           ` Igor Mammedov
2019-09-11 17:30                                                                             ` Laszlo Ersek [this message]
2019-09-17 13:11                                                                               ` [edk2-devel] " Igor Mammedov
2019-09-17 14:38                                                                                 ` [staging/branch]: CdePkg - C Development Environment Package Minnow Ware
2019-08-26 15:30                                                 ` [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF Laszlo Ersek
2019-08-27 16:23                                                   ` Igor Mammedov
2019-08-27 20:11                                                     ` Laszlo Ersek
2019-08-28 12:01                                                       ` Igor Mammedov
2019-08-29 16:25                                                         ` Laszlo Ersek
2019-08-30 13:49                                                           ` [Qemu-devel] " Igor Mammedov
2019-08-22 17:53                             ` Laszlo Ersek
2019-08-16 20:00           ` Laszlo Ersek
2019-08-15 16:07       ` Igor Mammedov
2019-08-15 16:24         ` Paolo Bonzini
2019-08-16  7:42           ` Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ed1ab55-d5ee-9703-dd28-cf50fbda5408@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox