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

[Michael Roth]

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 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.