Re: [Qemu-devel] [Qemu-ppc] [PATCH v7 0/2] spapr-rtas: add ibm, get-vpd RTAS interface

[David Gibson]

On Mon, Apr 08, 2019 at 05:34:48PM -0500, Michael Roth wrote:
> Quoting Greg Kurz (2019-04-08 11:31:56)
> > On Mon, 8 Apr 2019 14:21:50 +1000
> > David Gibson <address@hidden> wrote:
> > 
> > > On Fri, Mar 29, 2019 at 01:29:51PM +0100, Greg Kurz wrote:
> > > > On Thu, 28 Mar 2019 15:39:45 -0300
> > > > "Maxiwell S. Garcia" <address@hidden> wrote:
> > > >   
> > > > > Hi,
> > > > > 
> > > > > On Thu, Mar 28, 2019 at 02:21:51PM +0100, Greg Kurz wrote:  
> > > > > > On Wed, 27 Mar 2019 17:41:00 -0300
> > > > > > "Maxiwell S. Garcia" <address@hidden> wrote:
> > > > > >     
> > > > > > > Here are two patches to add a handler for ibm,get-vpd RTAS calls.
> > > > > > > This RTAS exposes host information in case of set QEMU options
> > > > > > > 'host-serial' and 'host-model' as 'passthrough'.
> > > > > > > 
> > > > > > > The patch 1 creates helper functions to get valid 'host-serial'
> > > > > > > and 'host-model' parameters, guided by QEMU command line. These
> > > > > > > parameters are useful to build the guest device tree and to return
> > > > > > > get-vpd RTAS calls. The patch 2 adds the ibm,get-vpd itself.
> > > > > > > 
> > > > > > > Update v7:
> > > > > > > * rtas_get_vpd_fields as a static array in spapr machine state
> > > > > > > 
> > > > > > > Maxiwell S. Garcia (2):
> > > > > > >   spapr: helper functions to get valid host fields
> > > > > > >   spapr-rtas: add ibm,get-vpd RTAS interface
> > > > > > > 
> > > > > > >  hw/ppc/spapr.c         | 48 +++++++++++----------
> > > > > > >  hw/ppc/spapr_rtas.c    | 96 
> > > > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  include/hw/ppc/spapr.h | 14 +++++-
> > > > > > >  3 files changed, 135 insertions(+), 23 deletions(-)
> > > > > > >     
> > > > > > 
> > > > > > Hi Maxiwell,
> > > > > > 
> > > > > > David sent a patch to rework how the host data is exposed to the 
> > > > > > guest.
> > > > > > Especially, the special casing of the "none" and "passthrough" 
> > > > > > strings
> > > > > > is no more... I'm afraid you'll have to rework your patches 
> > > > > > accordingly:
> > > > > > code+changelog in patch 1 and at least changelog in patch 2.
> > > > > > 
> > > > > > Cheers,    
> > > > > 
> > > > > IIUC, the 'ibm,get-vpd' RTAS should return information about the
> > > > > platform/cabinet. Thus, it's not necessary to add new nodes in the 
> > > > > guest
> > > > > device tree to export information like that.  
> > > > 
> > > > I agree that these "host-model" and "host-serial" props, which aren't
> > > > described anywhere and not used by either the linux kernel or the
> > > > powerpc-utils, look like a QEMU-specific poor man's version of VPD.
> > > > 
> > > > Not quite sure why they were even created since this is the purpose
> > > > of "system-id" and "model" as explained in PAPR, and supposedly
> > > > exposed in /proc/ppc64/lparcfg according to the LPARCFG(5) manual
> > > > page:  
> > > 
> > > Yeah, I'm not sure why they were created either.  I rather suspect
> > > nothing much is using them, and I'd kind of like to just kill them.
> > > But Daniel Berrange (and maybe others) are paranoid about this
> > > breaking things.
> > > 
> > 
> > Speaking of that. The "host-model"/"host-serial" fix is associated to a
> > CVE which affects QEMU versions currently shipped by downstream vendors.
> > Isn't a good enough reason to break things in existing unsecure setups ?
> > Should we add this patch to Mike's patch round-up for stable 3.0.1 (and
> > therefore break something that used to _work_ with 3.0.0) ?
> 
> Just for confirm: is the suggestion to backport 27461d69a? IIUC the fix
> would involve utilizing new command-line options to override the default
> "passthrough" mode for host-model/host-serial.
> 
> If so, I think an argument could be made, but I generally try to avoid
> anything relying on new command-line options since they're unlikely to be
> utilized unless the distro/vendor are likely to have specific plans to use
> them


Yeah :/

> and implement the appropriate changes elsewhere in their stack to do
> so (e.g.  stuff like Spectre mitigations). And, worst case, downstreams
> would still have the option of backporting the QEMU fixes as part of the
> overall CVE fix, so I'd probably opt to leave this one to the downstreams
> to consider.

So, in this case Openstack already does something similar for x86 - it
passes /etc/machine-id through the smbios information.  Unfortunately
the way it passes that down to qemu is via an explicit -smbios option.

It would be nice to have a common "host-id" or whatever option to qemu
which will work for both x86 via smbios and power via device tree
and/or get-vpd.  I had a look at that, and implementing it is fiddlier
than you'd think, because all the fallback logic and multiple pieces
to change (e.g. qemu would need to populate with explicit smbios info
first, then fall back to machine options, then libvirt would need a
common way to pass it through, then openstack would need to change to
use the common way, .... ugh).

At the moment this is in my "makes-my-brain-hurt-to-contemplate"
pile.  I'm open to suggestions.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature