Re: [Qemu-devel] [PATCH v3 for-2.9 0/3] q35: add negotiable broadcast SMI

[Igor Mammedov]

On Mon, 28 Nov 2016 10:41:42 +0100
Paolo Bonzini <address@hidden> wrote:

> On 25/11/2016 15:10, Igor Mammedov wrote:
> > On Fri, 25 Nov 2016 03:55:29 -0500 (EST)
> > Paolo Bonzini <address@hidden> wrote:  
> >>> if 0x30000 were covered by SMRR range, then OSPM wouldn't able to
> >>> place its own code there and there wouldn't be any need in side interfaces
> >>> to put and get CPU in/from some undefined by spec state (parked).
> >>>
> >>> But above would imply a large block allocated at 0x30000 to fit all
> >>> possible CPUs+1, not sure if it's doable (maybe edk2 wouldn't have
> >>> big issues with reserving large block in lowmem).    
> >>
> >> If you mean that the default SMRR range would include 0x30000 for an 
> >> hotplugged
> >> CPU, that would work but it is a significant departure from real hardware.
> >> I'd rather avoid that.  
> > 
> > But that's guest side only solution to guest problem, that won't require
> > any assistance from QEMU/KVM.  
> 
> No, I don't think it can be guest-only.  The initial value of SMRRs is
> undefined, and SMRRs are per processor.  The newly-hotplugged CPU has no
> SMRRs defined when it is started up.
Does it matter?
If 0x30000 is protected by SMRRs on exiting CPUs so OS can't tamper with it
and SMI is injected on hotplug
(i.e. hotplugged CPU arrives with SMI pin active, we can do this without
inventing PV solution to park/unpark CPU).

Then even if OS sends INIT/SIPI with code to hijack SMRAM that code
won't get a chance to run unrestricted as pending SMI will be processed
first and SMRAM will be relocated by firmware to per CPU area along with
configuring the hotplugged CPU's SMRR before malicious SIPI is even executed.


> > Baremetal would also benefit from it as it won't need to implement unpark
> > logic anymore. it should also work for existing HW that has unpark logic.
> > 
> > Do you have any pointers to how hardware does unparking now?  
> 
> The firmware tells the BMC to do it.  I don't know what the firmware-BMC
> interface looks like.
> 
> >>> It looks like we need only SMM accessible guest/host interface to make
> >>> CPU unparking secure or cover default SMBASE by SMRR.    
> >>
> >> Yes, unparking would be accessible only from SMM if the unparking feature
> >> is negotiated.  
> > 
> > I suppose we could check in MMIO handler that all CPUs are in SMM mode
> > before allowing unparking or ignore command if they are not.
> > 
> > For compat reasons unpark should be optin feature (i.e. firmware should
> > explicitly enable it to avoid breaking existing configurations (SeaBIOS))  
> 
> Yes, of course---that's why I'm bringing up in the context of this series.
> 
> Paolo