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

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

[Maxiwell S. Garcia]

Hi Greg,

Thanks for your review. I added some comments below...

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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
> 
>        serial_number
>        The serial number of the physical system in which the partition resides
> 
>        system_type
>        The  machine,type-model  of  the physical system in which the partition
>        resides
> 
> This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> 
> [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> lparcfg 1.9
> serial_number=IBM,032116A9A
> system_type=IBM,8247-22L
> 
> [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> IBM,032116A9A
> [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> IBM,8247-22L
> 
> But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> which is clearly not PAPR compliant according to this requirement:
> 
> 	R1–12.2–13. There must be a property, “model”, under the root node
> 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> 	one to five letters representing the stock symbol of the company
> 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> 	on page 343) of the first or ‘marked’ processor enclosure.
> 
> And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> identify guests, with "system-id" which is about the host :-\
> 
> All of this is very confusing and need to be sorted out before building
> anything on top of it. Especially since "model" and "system-id" are
> supposed to derive from VPD IIUC.
> 
> I guess that we should first decide what we really want to expose
> in "system-id" and "model": what we have now ? the same as in
> "host-serial" and "host-model", ie. user configurable ? Must we
> stay compatible with existing setups ? In any case, I'm afraid that
> we have to diverge from PAPR somehow, since it obviously doesn't
> care about the security concerns that motivated recent changes
> for "host-serial" and "host-model"...
> 

Many important changes should be done to solve these inconsistencies.
So, I saw in the 'get-vpd' RTAS a manner to return host information
and that works with live-migration.

> 
> > Since it's a POWER specific
> > functionality, may 'ibm,get-vpd' export host information if the
> > guest instance allows it? Or is it better return only the 'host-serial'
> > and 'host-model' content, like in the patch "spapr: Simplify handling
> > of host-serial and host-model values"?
> > 
> 
> I've spent some time reading PAPR on this topic and I'm not sure that
> "ibm,get-vpd" is the way to go for what you were trying to achieve
> initially (ie, obtain up-to-date host model and serial after migration).
> 
> Have you looked at RTAS "ibm,update-properties" ?

> 
> 	7.4.8 ibm,update-properties RTAS Call
> 
> 	This RTAS call is used to report updates to the properties changed
> 	due to a massive platform reconfiguration such as when the partition
> 	is migrated between machines.
> 
> This explicitly covers updates to "system-id" and "model". Maybe it is
> time to do as Ben was suggesting a long time ago when "host-serial"
> and "host-model" were introduced [1]:
> 
> 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:
> 	> Please be aware that all of the above is bogus when you start
> 	> thinking 
> 	> about live migration.
> 
> 	What's probably where we need to start thinking about implementing
> 	migration according to PAPR :-)
> 
> 	IE. With pre and post-migration notifications to the guest including
> 	device-tree updates.
> 
> 	Cheers,
> 	Ben.
> 
> [1] https://patchwork.ozlabs.org/patch/367792/#813547
> 
> Cheers,
> 

The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
nodes was modified. So, to implement this approach, the QEMU should change
the DT nodes when the live-migration finish and some service in the guest
need to call these RTAS to discovery what nodes was changed. Is it the
right way?

Assuming that in new pseries machine won't be possible choose
'passthrough' options to 'host-serial' and 'host-model' (last patch of
dgibson about this), it's necessary get the host information and set it
as string in these options. So, if we use the same qemu options in a
live-migration scenario for src and dst (libvirt do that), these
properties will remain the same. Is this behavior expected?

> --
> Greg
> 
> 
> > > 
> > > --
> > > Greg
> > >   
> > 
> > 
> 
> 

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

[Greg Kurz]

On Mon, 1 Apr 2019 12:01:48 -0300
"Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:

> Hi Greg,
> 
> Thanks for your review. I added some comments below...
> 
> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
> > 
> >        serial_number
> >        The serial number of the physical system in which the partition resides
> > 
> >        system_type
> >        The  machine,type-model  of  the physical system in which the partition
> >        resides
> > 
> > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > 
> > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > lparcfg 1.9
> > serial_number=IBM,032116A9A
> > system_type=IBM,8247-22L
> > 
> > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > IBM,032116A9A
> > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > IBM,8247-22L
> > 
> > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > which is clearly not PAPR compliant according to this requirement:
> > 
> > 	R1–12.2–13. There must be a property, “model”, under the root node
> > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > 	one to five letters representing the stock symbol of the company
> > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > 	on page 343) of the first or ‘marked’ processor enclosure.
> > 
> > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > identify guests, with "system-id" which is about the host :-\
> > 
> > All of this is very confusing and need to be sorted out before building
> > anything on top of it. Especially since "model" and "system-id" are
> > supposed to derive from VPD IIUC.
> > 
> > I guess that we should first decide what we really want to expose
> > in "system-id" and "model": what we have now ? the same as in
> > "host-serial" and "host-model", ie. user configurable ? Must we
> > stay compatible with existing setups ? In any case, I'm afraid that
> > we have to diverge from PAPR somehow, since it obviously doesn't
> > care about the security concerns that motivated recent changes
> > for "host-serial" and "host-model"...
> >   
> 
> Many important changes should be done to solve these inconsistencies.
> So, I saw in the 'get-vpd' RTAS a manner to return host information
> and that works with live-migration.
> 

Yes it does indeed allow that, among other things. My concern is more that
PAPR clearly indicates that the "system-id" and "model" in the root node
are derived from the VPD, and this series is about to tie the VPD TM and
VPD SE keywords to something else that isn't documented in PAPR...

The more I look at the "host-serial" and "host-model" properties, the more
I have the impression that they serve the same purpose as "system-id" and
"model" in PAPR (at least when peeking into the device tree of a PowerVM
LPAR as shown somewhere ^^)... what about unifying these ? It would likely
impact some guest software that use this to guess the platform type, like
powerpc-utils for example:

		} else if (strstr(line, "IBM pSeries (emulated by qemu)")) {
			rc = PLATFORM_POWERKVM_GUEST;
			break;
		} else if (strstr(line, "pSeries")) {

but this is fragile and should be improved anyway...

> >   
> > > Since it's a POWER specific
> > > functionality, may 'ibm,get-vpd' export host information if the
> > > guest instance allows it? Or is it better return only the 'host-serial'
> > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > of host-serial and host-model values"?
> > >   
> > 
> > I've spent some time reading PAPR on this topic and I'm not sure that
> > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > initially (ie, obtain up-to-date host model and serial after migration).
> > 
> > Have you looked at RTAS "ibm,update-properties" ?  
> 
> > 
> > 	7.4.8 ibm,update-properties RTAS Call
> > 
> > 	This RTAS call is used to report updates to the properties changed
> > 	due to a massive platform reconfiguration such as when the partition
> > 	is migrated between machines.
> > 
> > This explicitly covers updates to "system-id" and "model". Maybe it is
> > time to do as Ben was suggesting a long time ago when "host-serial"
> > and "host-model" were introduced [1]:
> > 
> > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > 	> Please be aware that all of the above is bogus when you start
> > 	> thinking 
> > 	> about live migration.  
> > 
> > 	What's probably where we need to start thinking about implementing
> > 	migration according to PAPR :-)
> > 
> > 	IE. With pre and post-migration notifications to the guest including
> > 	device-tree updates.
> > 
> > 	Cheers,
> > 	Ben.
> > 
> > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> > 
> > Cheers,
> >   
> 
> The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
> nodes was modified. So, to implement this approach, the QEMU should change
> the DT nodes when the live-migration finish and some service in the guest
> need to call these RTAS to discovery what nodes was changed. Is it the
> right way?
> 

If QEMU on the target is started with different "host-serial" and
"host-model", the DT in QEMU already has the new value. No need to
change anything. Appart from that, yes, QEMU should generate a PRRN
event at post load to notify the guest, which in turns do the RTAS
calls to get the updates.

> Assuming that in new pseries machine won't be possible choose
> 'passthrough' options to 'host-serial' and 'host-model' (last patch of
> dgibson about this), it's necessary get the host information and set it
> as string in these options. So, if we use the same qemu options in a
> live-migration scenario for src and dst (libvirt do that), these
> properties will remain the same. Is this behavior expected?
> 

The recent fixes around "host-serial" and "host-model" simply moved
the decision to expose host data to the upper layer, ie. libvirt
which should be involved in this discussion.

Cc'ing Andrea for expertise. Problem exposed below.

The pseries machine used to expose the content of the host's
/proc/device-tree/system-id and /proc/device-tree/model in the guest
DT. This led to a CVE and QEMU doesn't do that anymore for new machine
types. Instead, two new properties where added to the pseries machine:

pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
pseries-4.0.host-model=string (Host model to advertise in guest device tree)

It is up to the caller to pass something... which may be anything,
including something like $(cat /proc/device-tree/system-id) or
randomly generated.

Is there a chance libvirt can be taught to pass a different string
to the target QEMU in case of migration ?

> > --
> > Greg
> > 
> >   
> > > > 
> > > > --
> > > > Greg
> > > >     
> > > 
> > >   
> > 
> >   
> 
> 

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

[Daniel Henrique Barboza]

On 4/2/19 7:28 AM, Greg Kurz wrote:
> On Mon, 1 Apr 2019 12:01:48 -0300
> "Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:
>
>> Hi Greg,
>>
>> Thanks for your review. I added some comments below...
>>
>> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
>>>
>>>         serial_number
>>>         The serial number of the physical system in which the partition resides
>>>
>>>         system_type
>>>         The  machine,type-model  of  the physical system in which the partition
>>>         resides
>>>
>>> This is indeed what we get in a PowerVM LPAR running on a tuleta system:
>>>
>>> [root@furax1 ~]# head -3 /proc/ppc64/lparcfg
>>> lparcfg 1.9
>>> serial_number=IBM,032116A9A
>>> system_type=IBM,8247-22L
>>>
>>> [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
>>> IBM,032116A9A
>>> [root@furax1 ~]# echo $(cat /proc/device-tree/model)
>>> IBM,8247-22L
>>>
>>> But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
>>> which is clearly not PAPR compliant according to this requirement:
>>>
>>> 	R1–12.2–13. There must be a property, “model”, under the root node
>>> 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
>>> 	one to five letters representing the stock symbol of the company
>>> 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
>>> 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
>>> 	on page 343) of the first or ‘marked’ processor enclosure.
>>>
>>> And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
>>> identify guests, with "system-id" which is about the host :-\
>>>
>>> All of this is very confusing and need to be sorted out before building
>>> anything on top of it. Especially since "model" and "system-id" are
>>> supposed to derive from VPD IIUC.
>>>
>>> I guess that we should first decide what we really want to expose
>>> in "system-id" and "model": what we have now ? the same as in
>>> "host-serial" and "host-model", ie. user configurable ? Must we
>>> stay compatible with existing setups ? In any case, I'm afraid that
>>> we have to diverge from PAPR somehow, since it obviously doesn't
>>> care about the security concerns that motivated recent changes
>>> for "host-serial" and "host-model"...
>>>    
>> Many important changes should be done to solve these inconsistencies.
>> So, I saw in the 'get-vpd' RTAS a manner to return host information
>> and that works with live-migration.
>>
> Yes it does indeed allow that, among other things. My concern is more that
> PAPR clearly indicates that the "system-id" and "model" in the root node
> are derived from the VPD, and this series is about to tie the VPD TM and
> VPD SE keywords to something else that isn't documented in PAPR...
>
> The more I look at the "host-serial" and "host-model" properties, the more
> I have the impression that they serve the same purpose as "system-id" and
> "model" in PAPR (at least when peeking into the device tree of a PowerVM
> LPAR as shown somewhere ^^)... what about unifying these ? It would likely
> impact some guest software that use this to guess the platform type, like
> powerpc-utils for example:
>
> 		} else if (strstr(line, "IBM pSeries (emulated by qemu)")) {
> 			rc = PLATFORM_POWERKVM_GUEST;
> 			break;
> 		} else if (strstr(line, "pSeries")) {
>
> but this is fragile and should be improved anyway...
>
>>>    
>>>> Since it's a POWER specific
>>>> functionality, may 'ibm,get-vpd' export host information if the
>>>> guest instance allows it? Or is it better return only the 'host-serial'
>>>> and 'host-model' content, like in the patch "spapr: Simplify handling
>>>> of host-serial and host-model values"?
>>>>    
>>> I've spent some time reading PAPR on this topic and I'm not sure that
>>> "ibm,get-vpd" is the way to go for what you were trying to achieve
>>> initially (ie, obtain up-to-date host model and serial after migration).
>>>
>>> Have you looked at RTAS "ibm,update-properties" ?
>>> 	7.4.8 ibm,update-properties RTAS Call
>>>
>>> 	This RTAS call is used to report updates to the properties changed
>>> 	due to a massive platform reconfiguration such as when the partition
>>> 	is migrated between machines.
>>>
>>> This explicitly covers updates to "system-id" and "model". Maybe it is
>>> time to do as Ben was suggesting a long time ago when "host-serial"
>>> and "host-model" were introduced [1]:
>>>
>>> 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:
>>> 	> Please be aware that all of the above is bogus when you start
>>> 	> thinking
>>> 	> about live migration.
>>>
>>> 	What's probably where we need to start thinking about implementing
>>> 	migration according to PAPR :-)
>>>
>>> 	IE. With pre and post-migration notifications to the guest including
>>> 	device-tree updates.
>>>
>>> 	Cheers,
>>> 	Ben.
>>>
>>> [1] https://patchwork.ozlabs.org/patch/367792/#813547
>>>
>>> Cheers,
>>>    
>> The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
>> nodes was modified. So, to implement this approach, the QEMU should change
>> the DT nodes when the live-migration finish and some service in the guest
>> need to call these RTAS to discovery what nodes was changed. Is it the
>> right way?
>>
> If QEMU on the target is started with different "host-serial" and
> "host-model", the DT in QEMU already has the new value. No need to
> change anything. Appart from that, yes, QEMU should generate a PRRN
> event at post load to notify the guest, which in turns do the RTAS
> calls to get the updates.
>
>> Assuming that in new pseries machine won't be possible choose
>> 'passthrough' options to 'host-serial' and 'host-model' (last patch of
>> dgibson about this), it's necessary get the host information and set it
>> as string in these options. So, if we use the same qemu options in a
>> live-migration scenario for src and dst (libvirt do that), these
>> properties will remain the same. Is this behavior expected?
>>
> The recent fixes around "host-serial" and "host-model" simply moved
> the decision to expose host data to the upper layer, ie. libvirt
> which should be involved in this discussion.
>
> Cc'ing Andrea for expertise. Problem exposed below.
>
> The pseries machine used to expose the content of the host's
> /proc/device-tree/system-id and /proc/device-tree/model in the guest
> DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> types. Instead, two new properties where added to the pseries machine:
>
> pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> pseries-4.0.host-model=string (Host model to advertise in guest device tree)
>
> It is up to the caller to pass something... which may be anything,
> including something like $(cat /proc/device-tree/system-id) or
> randomly generated.
>
> Is there a chance libvirt can be taught to pass a different string
> to the target QEMU in case of migration ?


Isn't that just pushing up the same problem (in this case, CVE-2019-8934)
to Libvirt? By the CVE description [1], the problem is exactly the exposure
of /proc/device-tree/system-id and /proc/device-tree/model system attributes
in the guest. The only difference is that instead of QEMU doing it, Libvirt
wll end up doing it if anything meaninful (i.e. anything that can identify
the host) is passed in host-serial in the destination during a migration.

I am no security expert (and please correct me if I'm wrong), but in the 
end,
what this CVE is telling is "the guest shouldn't know any host details 
or anything
that can uniquely identify the host". This means that we can't populate 
these
attributes with meaninful host data (serial number, hostname, etc).

This kind of obliterates the point of Max patch - the idea is that the 
user wants
to be able do pinpoint, after migrating, which host is running the VM, by
checking host_serial. Perhaps we should just discourage this usage 
altogether
and show this CVE as proof. The user will need to go to the VM management
layer to discover where the guest is (which is the proper way of doing it,
anyway).


Thanks,


DHB


[1] https://access.redhat.com/security/cve/cve-2019-8934



>
>>> --
>>> Greg
>>>
>>>    
>>>>> --
>>>>> Greg
>>>>>      
>>>>    
>>>    
>>
>

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

[Greg Kurz]

On Wed, 3 Apr 2019 18:07:24 -0300
Daniel Henrique Barboza <danielhb413@gmail.com> wrote:

> On 4/2/19 7:28 AM, Greg Kurz wrote:
> > On Mon, 1 Apr 2019 12:01:48 -0300
> > "Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:
> >  
> >> Hi Greg,
> >>
> >> Thanks for your review. I added some comments below...
> >>
> >> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
> >>>
> >>>         serial_number
> >>>         The serial number of the physical system in which the partition resides
> >>>
> >>>         system_type
> >>>         The  machine,type-model  of  the physical system in which the partition
> >>>         resides
> >>>
> >>> This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> >>>
> >>> [root@furax1 ~]# head -3 /proc/ppc64/lparcfg
> >>> lparcfg 1.9
> >>> serial_number=IBM,032116A9A
> >>> system_type=IBM,8247-22L
> >>>
> >>> [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> >>> IBM,032116A9A
> >>> [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> >>> IBM,8247-22L
> >>>
> >>> But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> >>> which is clearly not PAPR compliant according to this requirement:
> >>>
> >>> 	R1–12.2–13. There must be a property, “model”, under the root node
> >>> 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> >>> 	one to five letters representing the stock symbol of the company
> >>> 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> >>> 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> >>> 	on page 343) of the first or ‘marked’ processor enclosure.
> >>>
> >>> And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> >>> identify guests, with "system-id" which is about the host :-\
> >>>
> >>> All of this is very confusing and need to be sorted out before building
> >>> anything on top of it. Especially since "model" and "system-id" are
> >>> supposed to derive from VPD IIUC.
> >>>
> >>> I guess that we should first decide what we really want to expose
> >>> in "system-id" and "model": what we have now ? the same as in
> >>> "host-serial" and "host-model", ie. user configurable ? Must we
> >>> stay compatible with existing setups ? In any case, I'm afraid that
> >>> we have to diverge from PAPR somehow, since it obviously doesn't
> >>> care about the security concerns that motivated recent changes
> >>> for "host-serial" and "host-model"...
> >>>      
> >> Many important changes should be done to solve these inconsistencies.
> >> So, I saw in the 'get-vpd' RTAS a manner to return host information
> >> and that works with live-migration.
> >>  
> > Yes it does indeed allow that, among other things. My concern is more that
> > PAPR clearly indicates that the "system-id" and "model" in the root node
> > are derived from the VPD, and this series is about to tie the VPD TM and
> > VPD SE keywords to something else that isn't documented in PAPR...
> >
> > The more I look at the "host-serial" and "host-model" properties, the more
> > I have the impression that they serve the same purpose as "system-id" and
> > "model" in PAPR (at least when peeking into the device tree of a PowerVM
> > LPAR as shown somewhere ^^)... what about unifying these ? It would likely
> > impact some guest software that use this to guess the platform type, like
> > powerpc-utils for example:
> >
> > 		} else if (strstr(line, "IBM pSeries (emulated by qemu)")) {
> > 			rc = PLATFORM_POWERKVM_GUEST;
> > 			break;
> > 		} else if (strstr(line, "pSeries")) {
> >
> > but this is fragile and should be improved anyway...
> >  
> >>>      
> >>>> Since it's a POWER specific
> >>>> functionality, may 'ibm,get-vpd' export host information if the
> >>>> guest instance allows it? Or is it better return only the 'host-serial'
> >>>> and 'host-model' content, like in the patch "spapr: Simplify handling
> >>>> of host-serial and host-model values"?
> >>>>      
> >>> I've spent some time reading PAPR on this topic and I'm not sure that
> >>> "ibm,get-vpd" is the way to go for what you were trying to achieve
> >>> initially (ie, obtain up-to-date host model and serial after migration).
> >>>
> >>> Have you looked at RTAS "ibm,update-properties" ?
> >>> 	7.4.8 ibm,update-properties RTAS Call
> >>>
> >>> 	This RTAS call is used to report updates to the properties changed
> >>> 	due to a massive platform reconfiguration such as when the partition
> >>> 	is migrated between machines.
> >>>
> >>> This explicitly covers updates to "system-id" and "model". Maybe it is
> >>> time to do as Ben was suggesting a long time ago when "host-serial"
> >>> and "host-model" were introduced [1]:
> >>>
> >>> 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> >>> 	> Please be aware that all of the above is bogus when you start
> >>> 	> thinking
> >>> 	> about live migration.  
> >>>
> >>> 	What's probably where we need to start thinking about implementing
> >>> 	migration according to PAPR :-)
> >>>
> >>> 	IE. With pre and post-migration notifications to the guest including
> >>> 	device-tree updates.
> >>>
> >>> 	Cheers,
> >>> 	Ben.
> >>>
> >>> [1] https://patchwork.ozlabs.org/patch/367792/#813547
> >>>
> >>> Cheers,
> >>>      
> >> The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
> >> nodes was modified. So, to implement this approach, the QEMU should change
> >> the DT nodes when the live-migration finish and some service in the guest
> >> need to call these RTAS to discovery what nodes was changed. Is it the
> >> right way?
> >>  
> > If QEMU on the target is started with different "host-serial" and
> > "host-model", the DT in QEMU already has the new value. No need to
> > change anything. Appart from that, yes, QEMU should generate a PRRN
> > event at post load to notify the guest, which in turns do the RTAS
> > calls to get the updates.
> >  
> >> Assuming that in new pseries machine won't be possible choose
> >> 'passthrough' options to 'host-serial' and 'host-model' (last patch of
> >> dgibson about this), it's necessary get the host information and set it
> >> as string in these options. So, if we use the same qemu options in a
> >> live-migration scenario for src and dst (libvirt do that), these
> >> properties will remain the same. Is this behavior expected?
> >>  
> > The recent fixes around "host-serial" and "host-model" simply moved
> > the decision to expose host data to the upper layer, ie. libvirt
> > which should be involved in this discussion.
> >
> > Cc'ing Andrea for expertise. Problem exposed below.
> >
> > The pseries machine used to expose the content of the host's
> > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > types. Instead, two new properties where added to the pseries machine:
> >
> > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> >
> > It is up to the caller to pass something... which may be anything,
> > including something like $(cat /proc/device-tree/system-id) or
> > randomly generated.
> >
> > Is there a chance libvirt can be taught to pass a different string
> > to the target QEMU in case of migration ?  
> 
> 
> Isn't that just pushing up the same problem (in this case, CVE-2019-8934)
> to Libvirt? By the CVE description [1], the problem is exactly the exposure
> of /proc/device-tree/system-id and /proc/device-tree/model system attributes
> in the guest. The only difference is that instead of QEMU doing it, Libvirt
> wll end up doing it if anything meaninful (i.e. anything that can identify
> the host) is passed in host-serial in the destination during a migration.
> 
> I am no security expert (and please correct me if I'm wrong), but in the 
> end,
> what this CVE is telling is "the guest shouldn't know any host details 
> or anything
> that can uniquely identify the host". This means that we can't populate 
> these
> attributes with meaninful host data (serial number, hostname, etc).
> 

Not exactly IMHO. That CVE is telling that QEMU shouldn't expose host
details unconditionally to the guest without the user or management
layer consent. But as explained in the changelog of 0a794529bd11:

    This does mean that the "passthrough" functionality is no longer
    available with the current machine type.  That's ok though: if a
    user or management layer really wants the information passed
    through they can read it themselves (OpenStack Nova already does
    something similar for x86).

This is a matter of policy.

> This kind of obliterates the point of Max patch - the idea is that the 
> user wants

If user means the QEMU user, this isn't true. The idea behind Max's patch
is that some software within the guest may need to track the host serial
for some legit purpose, eg. hardware inventory or license validations
software as explained here:

https://bugs.launchpad.net/nova/+bug/1337349/comments/6

> to be able do pinpoint, after migrating, which host is running the VM, by
> checking host_serial. Perhaps we should just discourage this usage 
> altogether
> and show this CVE as proof. The user will need to go to the VM management
> layer to discover where the guest is (which is the proper way of doing it,
> anyway).
> 

I agree that exposing real host HW identifiers is not recommended, but
it is ok to expose virtualized ones. For example, the LP above mentions
systemd's /etc/machine-id UUID, which is what Nova now tries first in order
to provide a unique host identifier to the guest IIUC.

My concern is more about reconciling this with PAPR, which doesn't seem
to care that much about not exposing real HW serials or location code
to partitions.

Cheers,

--
Greg


> 
> Thanks,
> 
> 
> DHB
> 
> 
> [1] https://access.redhat.com/security/cve/cve-2019-8934
> 
> 
> 
> >  
> >>> --
> >>> Greg
> >>>
> >>>      
> >>>>> --
> >>>>> Greg
> >>>>>        
> >>>>      
> >>>      
> >>  
> >  
> 

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 10852 bytes --]

On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:
> On Mon, 1 Apr 2019 12:01:48 -0300
> "Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:
> 
> > Hi Greg,
> > 
> > Thanks for your review. I added some comments below...
> > 
> > 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
> > > 
> > >        serial_number
> > >        The serial number of the physical system in which the partition resides
> > > 
> > >        system_type
> > >        The  machine,type-model  of  the physical system in which the partition
> > >        resides
> > > 
> > > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > > 
> > > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > > lparcfg 1.9
> > > serial_number=IBM,032116A9A
> > > system_type=IBM,8247-22L
> > > 
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > > IBM,032116A9A
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > > IBM,8247-22L
> > > 
> > > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > > which is clearly not PAPR compliant according to this requirement:
> > > 
> > > 	R1–12.2–13. There must be a property, “model”, under the root node
> > > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > > 	one to five letters representing the stock symbol of the company
> > > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > > 	on page 343) of the first or ‘marked’ processor enclosure.
> > > 
> > > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > > identify guests, with "system-id" which is about the host :-\
> > > 
> > > All of this is very confusing and need to be sorted out before building
> > > anything on top of it. Especially since "model" and "system-id" are
> > > supposed to derive from VPD IIUC.
> > > 
> > > I guess that we should first decide what we really want to expose
> > > in "system-id" and "model": what we have now ? the same as in
> > > "host-serial" and "host-model", ie. user configurable ? Must we
> > > stay compatible with existing setups ? In any case, I'm afraid that
> > > we have to diverge from PAPR somehow, since it obviously doesn't
> > > care about the security concerns that motivated recent changes
> > > for "host-serial" and "host-model"...
> > >   
> > 
> > Many important changes should be done to solve these inconsistencies.
> > So, I saw in the 'get-vpd' RTAS a manner to return host information
> > and that works with live-migration.
> > 
> 
> Yes it does indeed allow that, among other things. My concern is more that
> PAPR clearly indicates that the "system-id" and "model" in the root node
> are derived from the VPD, and this series is about to tie the VPD TM and
> VPD SE keywords to something else that isn't documented in PAPR...
> 
> The more I look at the "host-serial" and "host-model" properties, the more
> I have the impression that they serve the same purpose as "system-id" and
> "model" in PAPR (at least when peeking into the device tree of a PowerVM
> LPAR as shown somewhere ^^)... what about unifying these ? It would likely
> impact some guest software that use this to guess the platform type, like
> powerpc-utils for example:
> 
> 		} else if (strstr(line, "IBM pSeries (emulated by qemu)")) {
> 			rc = PLATFORM_POWERKVM_GUEST;
> 			break;
> 		} else if (strstr(line, "pSeries")) {
> 
> but this is fragile and should be improved anyway...
> 
> > >   
> > > > Since it's a POWER specific
> > > > functionality, may 'ibm,get-vpd' export host information if the
> > > > guest instance allows it? Or is it better return only the 'host-serial'
> > > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > > of host-serial and host-model values"?
> > > >   
> > > 
> > > I've spent some time reading PAPR on this topic and I'm not sure that
> > > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > > initially (ie, obtain up-to-date host model and serial after migration).
> > > 
> > > Have you looked at RTAS "ibm,update-properties" ?  
> > 
> > > 
> > > 	7.4.8 ibm,update-properties RTAS Call
> > > 
> > > 	This RTAS call is used to report updates to the properties changed
> > > 	due to a massive platform reconfiguration such as when the partition
> > > 	is migrated between machines.
> > > 
> > > This explicitly covers updates to "system-id" and "model". Maybe it is
> > > time to do as Ben was suggesting a long time ago when "host-serial"
> > > and "host-model" were introduced [1]:
> > > 
> > > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > > 	> Please be aware that all of the above is bogus when you start
> > > 	> thinking 
> > > 	> about live migration.  
> > > 
> > > 	What's probably where we need to start thinking about implementing
> > > 	migration according to PAPR :-)
> > > 
> > > 	IE. With pre and post-migration notifications to the guest including
> > > 	device-tree updates.
> > > 
> > > 
> > > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> > > 
> > 
> > The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
> > nodes was modified. So, to implement this approach, the QEMU should change
> > the DT nodes when the live-migration finish and some service in the guest
> > need to call these RTAS to discovery what nodes was changed. Is it the
> > right way?
> > 
> 
> If QEMU on the target is started with different "host-serial" and
> "host-model", the DT in QEMU already has the new value. No need to
> change anything. Appart from that, yes, QEMU should generate a PRRN
> event at post load to notify the guest, which in turns do the RTAS
> calls to get the updates.

Hrm.  The way the DT is handled between qemu, SLOF and the guest
would, I suspect, make ibm,update-properties a serious PITA to
implement.  So, I'm not super keen on that idea.

More generally, I'm not sure merging concepts from PAPR's guest-aware
migration model into qemu's guest-unaware migration model is a great
idea.

> > Assuming that in new pseries machine won't be possible choose
> > 'passthrough' options to 'host-serial' and 'host-model' (last patch of
> > dgibson about this), it's necessary get the host information and set it
> > as string in these options. So, if we use the same qemu options in a
> > live-migration scenario for src and dst (libvirt do that), these
> > properties will remain the same. Is this behavior expected?
> > 
> 
> The recent fixes around "host-serial" and "host-model" simply moved
> the decision to expose host data to the upper layer, ie. libvirt
> which should be involved in this discussion.

Right, that's deliberate.  Note that roughly-equivalent information on
x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
rather than qemu, and I'd like to move towards a common configuration
model with x86, though it's a fairly long path to there.

OpenStack had an equivalent security problem to our one, which it
addressed by taking the host serial from /etc/machine-id if present
rather than the real host info.

> Cc'ing Andrea for expertise. Problem exposed below.
> 
> The pseries machine used to expose the content of the host's
> /proc/device-tree/system-id and /proc/device-tree/model in the guest
> DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> types. Instead, two new properties where added to the pseries machine:
> 
> pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> 
> It is up to the caller to pass something... which may be anything,
> including something like $(cat /proc/device-tree/system-id) or
> randomly generated.
> 
> Is there a chance libvirt can be taught to pass a different string
> to the target QEMU in case of migration ?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 10852 bytes --]

On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:
> On Mon, 1 Apr 2019 12:01:48 -0300
> "Maxiwell S. Garcia" <maxiwell@linux.ibm.com> wrote:
> 
> > Hi Greg,
> > 
> > Thanks for your review. I added some comments below...
> > 
> > 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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:
> > > 
> > >        serial_number
> > >        The serial number of the physical system in which the partition resides
> > > 
> > >        system_type
> > >        The  machine,type-model  of  the physical system in which the partition
> > >        resides
> > > 
> > > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > > 
> > > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > > lparcfg 1.9
> > > serial_number=IBM,032116A9A
> > > system_type=IBM,8247-22L
> > > 
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > > IBM,032116A9A
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > > IBM,8247-22L
> > > 
> > > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > > which is clearly not PAPR compliant according to this requirement:
> > > 
> > > 	R1–12.2–13. There must be a property, “model”, under the root node
> > > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > > 	one to five letters representing the stock symbol of the company
> > > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > > 	on page 343) of the first or ‘marked’ processor enclosure.
> > > 
> > > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > > identify guests, with "system-id" which is about the host :-\
> > > 
> > > All of this is very confusing and need to be sorted out before building
> > > anything on top of it. Especially since "model" and "system-id" are
> > > supposed to derive from VPD IIUC.
> > > 
> > > I guess that we should first decide what we really want to expose
> > > in "system-id" and "model": what we have now ? the same as in
> > > "host-serial" and "host-model", ie. user configurable ? Must we
> > > stay compatible with existing setups ? In any case, I'm afraid that
> > > we have to diverge from PAPR somehow, since it obviously doesn't
> > > care about the security concerns that motivated recent changes
> > > for "host-serial" and "host-model"...
> > >   
> > 
> > Many important changes should be done to solve these inconsistencies.
> > So, I saw in the 'get-vpd' RTAS a manner to return host information
> > and that works with live-migration.
> > 
> 
> Yes it does indeed allow that, among other things. My concern is more that
> PAPR clearly indicates that the "system-id" and "model" in the root node
> are derived from the VPD, and this series is about to tie the VPD TM and
> VPD SE keywords to something else that isn't documented in PAPR...
> 
> The more I look at the "host-serial" and "host-model" properties, the more
> I have the impression that they serve the same purpose as "system-id" and
> "model" in PAPR (at least when peeking into the device tree of a PowerVM
> LPAR as shown somewhere ^^)... what about unifying these ? It would likely
> impact some guest software that use this to guess the platform type, like
> powerpc-utils for example:
> 
> 		} else if (strstr(line, "IBM pSeries (emulated by qemu)")) {
> 			rc = PLATFORM_POWERKVM_GUEST;
> 			break;
> 		} else if (strstr(line, "pSeries")) {
> 
> but this is fragile and should be improved anyway...
> 
> > >   
> > > > Since it's a POWER specific
> > > > functionality, may 'ibm,get-vpd' export host information if the
> > > > guest instance allows it? Or is it better return only the 'host-serial'
> > > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > > of host-serial and host-model values"?
> > > >   
> > > 
> > > I've spent some time reading PAPR on this topic and I'm not sure that
> > > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > > initially (ie, obtain up-to-date host model and serial after migration).
> > > 
> > > Have you looked at RTAS "ibm,update-properties" ?  
> > 
> > > 
> > > 	7.4.8 ibm,update-properties RTAS Call
> > > 
> > > 	This RTAS call is used to report updates to the properties changed
> > > 	due to a massive platform reconfiguration such as when the partition
> > > 	is migrated between machines.
> > > 
> > > This explicitly covers updates to "system-id" and "model". Maybe it is
> > > time to do as Ben was suggesting a long time ago when "host-serial"
> > > and "host-model" were introduced [1]:
> > > 
> > > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > > 	> Please be aware that all of the above is bogus when you start
> > > 	> thinking 
> > > 	> about live migration.  
> > > 
> > > 	What's probably where we need to start thinking about implementing
> > > 	migration according to PAPR :-)
> > > 
> > > 	IE. With pre and post-migration notifications to the guest including
> > > 	device-tree updates.
> > > 
> > > 
> > > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> > > 
> > 
> > The 'ibm,update-properties' and 'ibm,update-nodes' RTAS report which DT
> > nodes was modified. So, to implement this approach, the QEMU should change
> > the DT nodes when the live-migration finish and some service in the guest
> > need to call these RTAS to discovery what nodes was changed. Is it the
> > right way?
> > 
> 
> If QEMU on the target is started with different "host-serial" and
> "host-model", the DT in QEMU already has the new value. No need to
> change anything. Appart from that, yes, QEMU should generate a PRRN
> event at post load to notify the guest, which in turns do the RTAS
> calls to get the updates.

Hrm.  The way the DT is handled between qemu, SLOF and the guest
would, I suspect, make ibm,update-properties a serious PITA to
implement.  So, I'm not super keen on that idea.

More generally, I'm not sure merging concepts from PAPR's guest-aware
migration model into qemu's guest-unaware migration model is a great
idea.

> > Assuming that in new pseries machine won't be possible choose
> > 'passthrough' options to 'host-serial' and 'host-model' (last patch of
> > dgibson about this), it's necessary get the host information and set it
> > as string in these options. So, if we use the same qemu options in a
> > live-migration scenario for src and dst (libvirt do that), these
> > properties will remain the same. Is this behavior expected?
> > 
> 
> The recent fixes around "host-serial" and "host-model" simply moved
> the decision to expose host data to the upper layer, ie. libvirt
> which should be involved in this discussion.

Right, that's deliberate.  Note that roughly-equivalent information on
x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
rather than qemu, and I'd like to move towards a common configuration
model with x86, though it's a fairly long path to there.

OpenStack had an equivalent security problem to our one, which it
addressed by taking the host serial from /etc/machine-id if present
rather than the real host info.

> Cc'ing Andrea for expertise. Problem exposed below.
> 
> The pseries machine used to expose the content of the host's
> /proc/device-tree/system-id and /proc/device-tree/model in the guest
> DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> types. Instead, two new properties where added to the pseries machine:
> 
> pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> 
> It is up to the caller to pass something... which may be anything,
> including something like $(cat /proc/device-tree/system-id) or
> randomly generated.
> 
> Is there a chance libvirt can be taught to pass a different string
> to the target QEMU in case of migration ?

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Andrea Bolognani]

Apologies for taking this long to respond.

On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:
> > The recent fixes around "host-serial" and "host-model" simply moved
> > the decision to expose host data to the upper layer, ie. libvirt
> > which should be involved in this discussion.
> 
> Right, that's deliberate.  Note that roughly-equivalent information on
> x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> rather than qemu, and I'd like to move towards a common configuration
> model with x86, though it's a fairly long path to there.
> 
> OpenStack had an equivalent security problem to our one, which it
> addressed by taking the host serial from /etc/machine-id if present
> rather than the real host info.

IIUC the situation is a bit different between x86 and ppc64, because
while for the latter SPAPR defines a way for the guest to access
information about the host it's running on, that's not the case for
the former, at least to the best of my knowledge.

What OpenStack is doing is reading the machine-id (if explicitly
configured to do so: the default is to use the guest's own UUID[1])
and exposing that as the *guest* serial, not as the *host* serial.

>From libvirt's point of view, the entire mechanism is entirely
optional, so unless the management layer explicitly asks it to set
a certain value for the serial, libvirt will simply pass no
information down to QEMU.

The relevant XML elements[2] are clearly modeled after x86, so I
wonder if Nova is setting them also on ppc64 and if so, what the
guest will ultimately see...

> > Cc'ing Andrea for expertise. Problem exposed below.
> > 
> > The pseries machine used to expose the content of the host's
> > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > types. Instead, two new properties where added to the pseries machine:
> > 
> > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> > 
> > It is up to the caller to pass something... which may be anything,
> > including something like $(cat /proc/device-tree/system-id) or
> > randomly generated.

What happens if the caller doesn't provide any value? Will QEMU come
up with something itself?

Adding a few extra knobs in the vein as the existing ones sounds like
a fairly reasonable idea. It will still be up to the management layer
to actually provide the values.

> > Is there a chance libvirt can be taught to pass a different string
> > to the target QEMU in case of migration ?

libvirt already supports providing a different XML to the target
host, so changing a couple values should be no big deal.


As a final note, unless I've gotten it wrong and x86 actually *does*
provide a way for the guest to figure out its host's serial, then any
software relying on the attributes defined by SPAPR is ultimately not
portable to non-ppc64 hardware and should probably be rearchitected
to go through the management layer, as Daniel was also suggesting
earlier in the thread.


[1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L364-L372
[2] https://libvirt.org/formatdomain.html#elementsSysinfo
-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Andrea Bolognani]

Apologies for taking this long to respond.

On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:
> > The recent fixes around "host-serial" and "host-model" simply moved
> > the decision to expose host data to the upper layer, ie. libvirt
> > which should be involved in this discussion.
> 
> Right, that's deliberate.  Note that roughly-equivalent information on
> x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> rather than qemu, and I'd like to move towards a common configuration
> model with x86, though it's a fairly long path to there.
> 
> OpenStack had an equivalent security problem to our one, which it
> addressed by taking the host serial from /etc/machine-id if present
> rather than the real host info.

IIUC the situation is a bit different between x86 and ppc64, because
while for the latter SPAPR defines a way for the guest to access
information about the host it's running on, that's not the case for
the former, at least to the best of my knowledge.

What OpenStack is doing is reading the machine-id (if explicitly
configured to do so: the default is to use the guest's own UUID[1])
and exposing that as the *guest* serial, not as the *host* serial.

From libvirt's point of view, the entire mechanism is entirely
optional, so unless the management layer explicitly asks it to set
a certain value for the serial, libvirt will simply pass no
information down to QEMU.

The relevant XML elements[2] are clearly modeled after x86, so I
wonder if Nova is setting them also on ppc64 and if so, what the
guest will ultimately see...

> > Cc'ing Andrea for expertise. Problem exposed below.
> > 
> > The pseries machine used to expose the content of the host's
> > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > types. Instead, two new properties where added to the pseries machine:
> > 
> > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> > 
> > It is up to the caller to pass something... which may be anything,
> > including something like $(cat /proc/device-tree/system-id) or
> > randomly generated.

What happens if the caller doesn't provide any value? Will QEMU come
up with something itself?

Adding a few extra knobs in the vein as the existing ones sounds like
a fairly reasonable idea. It will still be up to the management layer
to actually provide the values.

> > Is there a chance libvirt can be taught to pass a different string
> > to the target QEMU in case of migration ?

libvirt already supports providing a different XML to the target
host, so changing a couple values should be no big deal.


As a final note, unless I've gotten it wrong and x86 actually *does*
provide a way for the guest to figure out its host's serial, then any
software relying on the attributes defined by SPAPR is ultimately not
portable to non-ppc64 hardware and should probably be rearchitected
to go through the management layer, as Daniel was also suggesting
earlier in the thread.


[1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L364-L372
[2] https://libvirt.org/formatdomain.html#elementsSysinfo
-- 
Andrea Bolognani / Red Hat / Virtualization

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

[Greg Kurz]

On Tue, 09 Apr 2019 18:24:07 +0200
Andrea Bolognani <abologna@redhat.com> wrote:

> Apologies for taking this long to respond.
> 

No problem :)

> On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> > On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:  
> > > The recent fixes around "host-serial" and "host-model" simply moved
> > > the decision to expose host data to the upper layer, ie. libvirt
> > > which should be involved in this discussion.  
> > 
> > Right, that's deliberate.  Note that roughly-equivalent information on
> > x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> > rather than qemu, and I'd like to move towards a common configuration
> > model with x86, though it's a fairly long path to there.
> > 
> > OpenStack had an equivalent security problem to our one, which it
> > addressed by taking the host serial from /etc/machine-id if present
> > rather than the real host info.  
> 
> IIUC the situation is a bit different between x86 and ppc64, because
> while for the latter SPAPR defines a way for the guest to access
> information about the host it's running on, that's not the case for
> the former, at least to the best of my knowledge.
> 
> What OpenStack is doing is reading the machine-id (if explicitly
> configured to do so: the default is to use the guest's own UUID[1])
> and exposing that as the *guest* serial, not as the *host* serial.
> 

Hmm... are you sure ? Daniel seems to be saying the opposite here:

https://bugs.launchpad.net/nova/+bug/1337349/comments/9

which was referring to:

https://bugs.launchpad.net/nova/+bug/1337349/comments/6

> From libvirt's point of view, the entire mechanism is entirely
> optional, so unless the management layer explicitly asks it to set
> a certain value for the serial, libvirt will simply pass no
> information down to QEMU.
> 
> The relevant XML elements[2] are clearly modeled after x86, so I
> wonder if Nova is setting them also on ppc64 and if so, what the
> guest will ultimately see...
> 
> > > Cc'ing Andrea for expertise. Problem exposed below.
> > > 
> > > The pseries machine used to expose the content of the host's
> > > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > > types. Instead, two new properties where added to the pseries machine:
> > > 
> > > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> > > 
> > > It is up to the caller to pass something... which may be anything,
> > > including something like $(cat /proc/device-tree/system-id) or
> > > randomly generated.  
> 
> What happens if the caller doesn't provide any value? Will QEMU come
> up with something itself?
> 

QEMU up to 3.1 used to fill these up with values from directly copied
from the host's device tree. Starting with 4.0, it is up to the caller
to provide values, with no fallback in QEMU.

> Adding a few extra knobs in the vein as the existing ones sounds like
> a fairly reasonable idea. It will still be up to the management layer
> to actually provide the values.
> 

Sure.

> > > Is there a chance libvirt can be taught to pass a different string
> > > to the target QEMU in case of migration ?  
> 
> libvirt already supports providing a different XML to the target
> host, so changing a couple values should be no big deal.
> 

Oh, I didn't know that, but that's cool. It is likely needed for
what Maxiwell's trying to achieve.

> 
> As a final note, unless I've gotten it wrong and x86 actually *does*
> provide a way for the guest to figure out its host's serial, then any
> software relying on the attributes defined by SPAPR is ultimately not
> portable to non-ppc64 hardware and should probably be rearchitected
> to go through the management layer, as Daniel was also suggesting
> earlier in the thread.
> 
> 
> [1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L364-L372
> [2] https://libvirt.org/formatdomain.html#elementsSysinfo

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

[Greg Kurz]

On Tue, 09 Apr 2019 18:24:07 +0200
Andrea Bolognani <abologna@redhat.com> wrote:

> Apologies for taking this long to respond.
> 

No problem :)

> On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> > On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:  
> > > The recent fixes around "host-serial" and "host-model" simply moved
> > > the decision to expose host data to the upper layer, ie. libvirt
> > > which should be involved in this discussion.  
> > 
> > Right, that's deliberate.  Note that roughly-equivalent information on
> > x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> > rather than qemu, and I'd like to move towards a common configuration
> > model with x86, though it's a fairly long path to there.
> > 
> > OpenStack had an equivalent security problem to our one, which it
> > addressed by taking the host serial from /etc/machine-id if present
> > rather than the real host info.  
> 
> IIUC the situation is a bit different between x86 and ppc64, because
> while for the latter SPAPR defines a way for the guest to access
> information about the host it's running on, that's not the case for
> the former, at least to the best of my knowledge.
> 
> What OpenStack is doing is reading the machine-id (if explicitly
> configured to do so: the default is to use the guest's own UUID[1])
> and exposing that as the *guest* serial, not as the *host* serial.
> 

Hmm... are you sure ? Daniel seems to be saying the opposite here:

https://bugs.launchpad.net/nova/+bug/1337349/comments/9

which was referring to:

https://bugs.launchpad.net/nova/+bug/1337349/comments/6

> From libvirt's point of view, the entire mechanism is entirely
> optional, so unless the management layer explicitly asks it to set
> a certain value for the serial, libvirt will simply pass no
> information down to QEMU.
> 
> The relevant XML elements[2] are clearly modeled after x86, so I
> wonder if Nova is setting them also on ppc64 and if so, what the
> guest will ultimately see...
> 
> > > Cc'ing Andrea for expertise. Problem exposed below.
> > > 
> > > The pseries machine used to expose the content of the host's
> > > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > > types. Instead, two new properties where added to the pseries machine:
> > > 
> > > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> > > 
> > > It is up to the caller to pass something... which may be anything,
> > > including something like $(cat /proc/device-tree/system-id) or
> > > randomly generated.  
> 
> What happens if the caller doesn't provide any value? Will QEMU come
> up with something itself?
> 

QEMU up to 3.1 used to fill these up with values from directly copied
from the host's device tree. Starting with 4.0, it is up to the caller
to provide values, with no fallback in QEMU.

> Adding a few extra knobs in the vein as the existing ones sounds like
> a fairly reasonable idea. It will still be up to the management layer
> to actually provide the values.
> 

Sure.

> > > Is there a chance libvirt can be taught to pass a different string
> > > to the target QEMU in case of migration ?  
> 
> libvirt already supports providing a different XML to the target
> host, so changing a couple values should be no big deal.
> 

Oh, I didn't know that, but that's cool. It is likely needed for
what Maxiwell's trying to achieve.

> 
> As a final note, unless I've gotten it wrong and x86 actually *does*
> provide a way for the guest to figure out its host's serial, then any
> software relying on the attributes defined by SPAPR is ultimately not
> portable to non-ppc64 hardware and should probably be rearchitected
> to go through the management layer, as Daniel was also suggesting
> earlier in the thread.
> 
> 
> [1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L364-L372
> [2] https://libvirt.org/formatdomain.html#elementsSysinfo

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

[Daniel P. Berrangé]

On Fri, Apr 12, 2019 at 04:57:01PM +0200, Greg Kurz wrote:
> On Tue, 09 Apr 2019 18:24:07 +0200
> Andrea Bolognani <abologna@redhat.com> wrote:
> 
> > Apologies for taking this long to respond.
> > 
> 
> No problem :)
> 
> > On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> > > On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:  
> > > > The recent fixes around "host-serial" and "host-model" simply moved
> > > > the decision to expose host data to the upper layer, ie. libvirt
> > > > which should be involved in this discussion.  
> > > 
> > > Right, that's deliberate.  Note that roughly-equivalent information on
> > > x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> > > rather than qemu, and I'd like to move towards a common configuration
> > > model with x86, though it's a fairly long path to there.
> > > 
> > > OpenStack had an equivalent security problem to our one, which it
> > > addressed by taking the host serial from /etc/machine-id if present
> > > rather than the real host info.  
> > 
> > IIUC the situation is a bit different between x86 and ppc64, because
> > while for the latter SPAPR defines a way for the guest to access
> > information about the host it's running on, that's not the case for
> > the former, at least to the best of my knowledge.
> > 
> > What OpenStack is doing is reading the machine-id (if explicitly
> > configured to do so: the default is to use the guest's own UUID[1])
> > and exposing that as the *guest* serial, not as the *host* serial.
> > 
> 
> Hmm... are you sure ? Daniel seems to be saying the opposite here:
> 
> https://bugs.launchpad.net/nova/+bug/1337349/comments/9

Note my comment is from 2014, where as Andrea is describing OpenStack's
impl in 2019 :-)

Originally Nova would populate guest SMBIOS with a UUID taken
from Host SMBIOS

My fix for the security problem made it use host /etc/machine-id
instead of Host SMBIOS when /etc/machine-id is available.

In Nova 2018, Nova was changed to use the guest instance UUID
instead of /etc/machine-id by default.

Anyway the key factor is to *never* expose and UUID you get
from the host hardware as vendors frequently treat these as
semi-secret keys.

If you need a UUID, it has to be software generated from
somewhere and not otherwise need to be kept private.

As long as QEMU lets this be configurable, libvirt can plumb
it into its sysinfo XML and thus we can ultimately delegate
the decision to the mgmt app like OpenStack.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Daniel P. Berrangé]

On Fri, Apr 12, 2019 at 04:57:01PM +0200, Greg Kurz wrote:
> On Tue, 09 Apr 2019 18:24:07 +0200
> Andrea Bolognani <abologna@redhat.com> wrote:
> 
> > Apologies for taking this long to respond.
> > 
> 
> No problem :)
> 
> > On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> > > On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:  
> > > > The recent fixes around "host-serial" and "host-model" simply moved
> > > > the decision to expose host data to the upper layer, ie. libvirt
> > > > which should be involved in this discussion.  
> > > 
> > > Right, that's deliberate.  Note that roughly-equivalent information on
> > > x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> > > rather than qemu, and I'd like to move towards a common configuration
> > > model with x86, though it's a fairly long path to there.
> > > 
> > > OpenStack had an equivalent security problem to our one, which it
> > > addressed by taking the host serial from /etc/machine-id if present
> > > rather than the real host info.  
> > 
> > IIUC the situation is a bit different between x86 and ppc64, because
> > while for the latter SPAPR defines a way for the guest to access
> > information about the host it's running on, that's not the case for
> > the former, at least to the best of my knowledge.
> > 
> > What OpenStack is doing is reading the machine-id (if explicitly
> > configured to do so: the default is to use the guest's own UUID[1])
> > and exposing that as the *guest* serial, not as the *host* serial.
> > 
> 
> Hmm... are you sure ? Daniel seems to be saying the opposite here:
> 
> https://bugs.launchpad.net/nova/+bug/1337349/comments/9

Note my comment is from 2014, where as Andrea is describing OpenStack's
impl in 2019 :-)

Originally Nova would populate guest SMBIOS with a UUID taken
from Host SMBIOS

My fix for the security problem made it use host /etc/machine-id
instead of Host SMBIOS when /etc/machine-id is available.

In Nova 2018, Nova was changed to use the guest instance UUID
instead of /etc/machine-id by default.

Anyway the key factor is to *never* expose and UUID you get
from the host hardware as vendors frequently treat these as
semi-secret keys.

If you need a UUID, it has to be software generated from
somewhere and not otherwise need to be kept private.

As long as QEMU lets this be configurable, libvirt can plumb
it into its sysinfo XML and thus we can ultimately delegate
the decision to the mgmt app like OpenStack.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4467 bytes --]

On Tue, Apr 09, 2019 at 06:24:07PM +0200, Andrea Bolognani wrote:
> Apologies for taking this long to respond.
> 
> On Mon, 2019-04-08 at 14:27 +1000, David Gibson wrote:
> > On Tue, Apr 02, 2019 at 12:28:07PM +0200, Greg Kurz wrote:
> > > The recent fixes around "host-serial" and "host-model" simply moved
> > > the decision to expose host data to the upper layer, ie. libvirt
> > > which should be involved in this discussion.
> > 
> > Right, that's deliberate.  Note that roughly-equivalent information on
> > x86 is currently supplied via the SMBIOS.  OpenStack Nova sets that,
> > rather than qemu, and I'd like to move towards a common configuration
> > model with x86, though it's a fairly long path to there.
> > 
> > OpenStack had an equivalent security problem to our one, which it
> > addressed by taking the host serial from /etc/machine-id if present
> > rather than the real host info.
> 
> IIUC the situation is a bit different between x86 and ppc64, because
> while for the latter SPAPR defines a way for the guest to access
> information about the host it's running on, that's not the case for
> the former, at least to the best of my knowledge.

Well, guests are getting this information in smbios somehow.  I don't
know what specifies that what bit of info in the smbios is host info,
but I guess it must at least be a de facto convention?

> What OpenStack is doing is reading the machine-id (if explicitly
> configured to do so: the default is to use the guest's own UUID[1])
> and exposing that as the *guest* serial, not as the *host* serial.

Uh.. I'm not sure about that.  I think the default is just to pass
nothing here.  The confusion is increased because this host info was
always handled at the above-libvirt (openstack) level on x86, using
explicit smbios configuration options down to libvirt and qemu.

On power we handled it in qemu, because that seemed the obvious way,
but it makes recombining them a pain.

> >From libvirt's point of view, the entire mechanism is entirely
> optional, so unless the management layer explicitly asks it to set
> a certain value for the serial, libvirt will simply pass no
> information down to QEMU.
> 
> The relevant XML elements[2] are clearly modeled after x86, so I
> wonder if Nova is setting them also on ppc64 and if so, what the
> guest will ultimately see...
> 
> > > Cc'ing Andrea for expertise. Problem exposed below.
> > > 
> > > The pseries machine used to expose the content of the host's
> > > /proc/device-tree/system-id and /proc/device-tree/model in the guest
> > > DT. This led to a CVE and QEMU doesn't do that anymore for new machine
> > > types. Instead, two new properties where added to the pseries machine:
> > > 
> > > pseries-4.0.host-serial=string (Host serial number to advertise in guest device tree)
> > > pseries-4.0.host-model=string (Host model to advertise in guest device tree)
> > > 
> > > It is up to the caller to pass something... which may be anything,
> > > including something like $(cat /proc/device-tree/system-id) or
> > > randomly generated.
> 
> What happens if the caller doesn't provide any value? Will QEMU come
> up with something itself?

Currently, no, it will just omit the properties.

> Adding a few extra knobs in the vein as the existing ones sounds like
> a fairly reasonable idea. It will still be up to the management layer
> to actually provide the values.
> 
> > > Is there a chance libvirt can be taught to pass a different string
> > > to the target QEMU in case of migration ?
> 
> libvirt already supports providing a different XML to the target
> host, so changing a couple values should be no big deal.
> 
> 
> As a final note, unless I've gotten it wrong and x86 actually *does*
> provide a way for the guest to figure out its host's serial, then any
> software relying on the attributes defined by SPAPR is ultimately not
> portable to non-ppc64 hardware and should probably be rearchitected
> to go through the management layer, as Daniel was also suggesting
> earlier in the thread.
> 
> 
> [1] https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L364-L372
> [2] https://libvirt.org/formatdomain.html#elementsSysinfo

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7008 bytes --]

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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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.

> 
>        serial_number
>        The serial number of the physical system in which the partition resides
> 
>        system_type
>        The  machine,type-model  of  the physical system in which the partition
>        resides
> 
> This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> 
> [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> lparcfg 1.9
> serial_number=IBM,032116A9A
> system_type=IBM,8247-22L
> 
> [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> IBM,032116A9A
> [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> IBM,8247-22L
> 
> But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> which is clearly not PAPR compliant according to this requirement:
> 
> 	R1–12.2–13. There must be a property, “model”, under the root node
> 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> 	one to five letters representing the stock symbol of the company
> 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> 	on page 343) of the first or ‘marked’ processor enclosure.
> 
> And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> identify guests, with "system-id" which is about the host :-\

Ugh, right.  So, it's been this way for years, so it's clearly not
breaking things in practice.  Given that, it might be best to leave
it, even though it violates PAPR technically.

Frankly, providing information about the *guest* virtual model and
"serial number" =~= UUID makes more sense than providing information
about the host that might be security sensitive.

> All of this is very confusing and need to be sorted out before building
> anything on top of it. Especially since "model" and "system-id" are
> supposed to derive from VPD IIUC.
> 
> I guess that we should first decide what we really want to expose
> in "system-id" and "model": what we have now ? the same as in
> "host-serial" and "host-model", ie. user configurable ? Must we
> stay compatible with existing setups ? In any case, I'm afraid that
> we have to diverge from PAPR somehow, since it obviously doesn't
> care about the security concerns that motivated recent changes
> for "host-serial" and "host-model"...

We absolutely should not expose the real host information without the
user explicitly requesting it.

> > Since it's a POWER specific
> > functionality, may 'ibm,get-vpd' export host information if the
> > guest instance allows it? Or is it better return only the 'host-serial'
> > and 'host-model' content, like in the patch "spapr: Simplify handling
> > of host-serial and host-model values"?
> > 
> 
> I've spent some time reading PAPR on this topic and I'm not sure that
> "ibm,get-vpd" is the way to go for what you were trying to achieve
> initially (ie, obtain up-to-date host model and serial after migration).
> 
> Have you looked at RTAS "ibm,update-properties" ?
> 
> 	7.4.8 ibm,update-properties RTAS Call
> 
> 	This RTAS call is used to report updates to the properties changed
> 	due to a massive platform reconfiguration such as when the partition
> 	is migrated between machines.

Yeah.. the thing here is that partition migration kind of means
something different in PAPR than it does in the qemu world.  It uses a
guest-aware model of migration (which frankly I think is broken
verging on unworkable).  qemu and uses a guest-(mostly)-unaware
migration model.

> This explicitly covers updates to "system-id" and "model". Maybe it is
> time to do as Ben was suggesting a long time ago when "host-serial"
> and "host-model" were introduced [1]:
> 
> 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:
> 	> Please be aware that all of the above is bogus when you start
> 	> thinking 
> 	> about live migration.
> 
> 	What's probably where we need to start thinking about implementing
> 	migration according to PAPR :-)
> 
> 	IE. With pre and post-migration notifications to the guest including
> 	device-tree updates.
> 
> 
> [1] https://patchwork.ozlabs.org/patch/367792/#813547
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7008 bytes --]

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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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.

> 
>        serial_number
>        The serial number of the physical system in which the partition resides
> 
>        system_type
>        The  machine,type-model  of  the physical system in which the partition
>        resides
> 
> This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> 
> [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> lparcfg 1.9
> serial_number=IBM,032116A9A
> system_type=IBM,8247-22L
> 
> [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> IBM,032116A9A
> [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> IBM,8247-22L
> 
> But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> which is clearly not PAPR compliant according to this requirement:
> 
> 	R1–12.2–13. There must be a property, “model”, under the root node
> 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> 	one to five letters representing the stock symbol of the company
> 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> 	on page 343) of the first or ‘marked’ processor enclosure.
> 
> And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> identify guests, with "system-id" which is about the host :-\

Ugh, right.  So, it's been this way for years, so it's clearly not
breaking things in practice.  Given that, it might be best to leave
it, even though it violates PAPR technically.

Frankly, providing information about the *guest* virtual model and
"serial number" =~= UUID makes more sense than providing information
about the host that might be security sensitive.

> All of this is very confusing and need to be sorted out before building
> anything on top of it. Especially since "model" and "system-id" are
> supposed to derive from VPD IIUC.
> 
> I guess that we should first decide what we really want to expose
> in "system-id" and "model": what we have now ? the same as in
> "host-serial" and "host-model", ie. user configurable ? Must we
> stay compatible with existing setups ? In any case, I'm afraid that
> we have to diverge from PAPR somehow, since it obviously doesn't
> care about the security concerns that motivated recent changes
> for "host-serial" and "host-model"...

We absolutely should not expose the real host information without the
user explicitly requesting it.

> > Since it's a POWER specific
> > functionality, may 'ibm,get-vpd' export host information if the
> > guest instance allows it? Or is it better return only the 'host-serial'
> > and 'host-model' content, like in the patch "spapr: Simplify handling
> > of host-serial and host-model values"?
> > 
> 
> I've spent some time reading PAPR on this topic and I'm not sure that
> "ibm,get-vpd" is the way to go for what you were trying to achieve
> initially (ie, obtain up-to-date host model and serial after migration).
> 
> Have you looked at RTAS "ibm,update-properties" ?
> 
> 	7.4.8 ibm,update-properties RTAS Call
> 
> 	This RTAS call is used to report updates to the properties changed
> 	due to a massive platform reconfiguration such as when the partition
> 	is migrated between machines.

Yeah.. the thing here is that partition migration kind of means
something different in PAPR than it does in the qemu world.  It uses a
guest-aware model of migration (which frankly I think is broken
verging on unworkable).  qemu and uses a guest-(mostly)-unaware
migration model.

> This explicitly covers updates to "system-id" and "model". Maybe it is
> time to do as Ben was suggesting a long time ago when "host-serial"
> and "host-model" were introduced [1]:
> 
> 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:
> 	> Please be aware that all of the above is bogus when you start
> 	> thinking 
> 	> about live migration.
> 
> 	What's probably where we need to start thinking about implementing
> 	migration according to PAPR :-)
> 
> 	IE. With pre and post-migration notifications to the guest including
> 	device-tree updates.
> 
> 
> [1] https://patchwork.ozlabs.org/patch/367792/#813547
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 8675 bytes --]

On Mon, 8 Apr 2019 14:21:50 +1000
David Gibson <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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) ?

> > 
> >        serial_number
> >        The serial number of the physical system in which the partition resides
> > 
> >        system_type
> >        The  machine,type-model  of  the physical system in which the partition
> >        resides
> > 
> > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > 
> > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > lparcfg 1.9
> > serial_number=IBM,032116A9A
> > system_type=IBM,8247-22L
> > 
> > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > IBM,032116A9A
> > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > IBM,8247-22L
> > 
> > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > which is clearly not PAPR compliant according to this requirement:
> > 
> > 	R1–12.2–13. There must be a property, “model”, under the root node
> > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > 	one to five letters representing the stock symbol of the company
> > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > 	on page 343) of the first or ‘marked’ processor enclosure.
> > 
> > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > identify guests, with "system-id" which is about the host :-\  
> 
> Ugh, right.  So, it's been this way for years, so it's clearly not
> breaking things in practice.  Given that, it might be best to leave
> it, even though it violates PAPR technically.
> 
> Frankly, providing information about the *guest* virtual model and
> "serial number" =~= UUID makes more sense than providing information
> about the host that might be security sensitive.
> 

IIUC, the rationale of exposing to the guest something that uniquely
identifies the host is needed by some inventory or license validation
softwares [1]. So I'd say we need both a guest UUID, that doesn't change
during the VM lifetime, and host UUID that changes when migrating to
another host.

The fact that "system-id" == "vm,uuid" in the current code didn't
break anything would rather indicate that there are no inventory
or license validation softwares users yet in the KVM+PAPR+linux
ecosystem...

[1] https://bugs.launchpad.net/nova/+bug/1337349/comments/6

> > All of this is very confusing and need to be sorted out before building
> > anything on top of it. Especially since "model" and "system-id" are
> > supposed to derive from VPD IIUC.
> > 
> > I guess that we should first decide what we really want to expose
> > in "system-id" and "model": what we have now ? the same as in
> > "host-serial" and "host-model", ie. user configurable ? Must we
> > stay compatible with existing setups ? In any case, I'm afraid that
> > we have to diverge from PAPR somehow, since it obviously doesn't
> > care about the security concerns that motivated recent changes
> > for "host-serial" and "host-model"...  
> 
> We absolutely should not expose the real host information without the
> user explicitly requesting it.
> 

I agree, but it seems that we may need something in between: an ID that
uniquely identifies a given system but that doesn't reveal security
sensitive information.

Looking again at PAPR, nothing much is said on the format of "system-id"
or VPD SN. Only "model" is expected to follow some formatting rules:
<vendor>,xxxx-yyy.

For "system-id", this could be addressed pretty much like in OpenStack
with systemd's /etc/machine-id. For "model", I'm not sure...

> > > Since it's a POWER specific
> > > functionality, may 'ibm,get-vpd' export host information if the
> > > guest instance allows it? Or is it better return only the 'host-serial'
> > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > of host-serial and host-model values"?
> > >   
> > 
> > I've spent some time reading PAPR on this topic and I'm not sure that
> > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > initially (ie, obtain up-to-date host model and serial after migration).
> > 
> > Have you looked at RTAS "ibm,update-properties" ?
> > 
> > 	7.4.8 ibm,update-properties RTAS Call
> > 
> > 	This RTAS call is used to report updates to the properties changed
> > 	due to a massive platform reconfiguration such as when the partition
> > 	is migrated between machines.  
> 
> Yeah.. the thing here is that partition migration kind of means
> something different in PAPR than it does in the qemu world.  It uses a
> guest-aware model of migration (which frankly I think is broken
> verging on unworkable).  qemu and uses a guest-(mostly)-unaware
> migration model.
> 

Yeah...

> > This explicitly covers updates to "system-id" and "model". Maybe it is
> > time to do as Ben was suggesting a long time ago when "host-serial"
> > and "host-model" were introduced [1]:
> > 
> > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > 	> Please be aware that all of the above is bogus when you start
> > 	> thinking 
> > 	> about live migration.  
> > 
> > 	What's probably where we need to start thinking about implementing
> > 	migration according to PAPR :-)
> > 
> > 	IE. With pre and post-migration notifications to the guest including
> > 	device-tree updates.
> > 
> > 
> > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> >   
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 8675 bytes --]

On Mon, 8 Apr 2019 14:21:50 +1000
David Gibson <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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) ?

> > 
> >        serial_number
> >        The serial number of the physical system in which the partition resides
> > 
> >        system_type
> >        The  machine,type-model  of  the physical system in which the partition
> >        resides
> > 
> > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > 
> > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > lparcfg 1.9
> > serial_number=IBM,032116A9A
> > system_type=IBM,8247-22L
> > 
> > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > IBM,032116A9A
> > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > IBM,8247-22L
> > 
> > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > which is clearly not PAPR compliant according to this requirement:
> > 
> > 	R1–12.2–13. There must be a property, “model”, under the root node
> > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > 	one to five letters representing the stock symbol of the company
> > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > 	on page 343) of the first or ‘marked’ processor enclosure.
> > 
> > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > identify guests, with "system-id" which is about the host :-\  
> 
> Ugh, right.  So, it's been this way for years, so it's clearly not
> breaking things in practice.  Given that, it might be best to leave
> it, even though it violates PAPR technically.
> 
> Frankly, providing information about the *guest* virtual model and
> "serial number" =~= UUID makes more sense than providing information
> about the host that might be security sensitive.
> 

IIUC, the rationale of exposing to the guest something that uniquely
identifies the host is needed by some inventory or license validation
softwares [1]. So I'd say we need both a guest UUID, that doesn't change
during the VM lifetime, and host UUID that changes when migrating to
another host.

The fact that "system-id" == "vm,uuid" in the current code didn't
break anything would rather indicate that there are no inventory
or license validation softwares users yet in the KVM+PAPR+linux
ecosystem...

[1] https://bugs.launchpad.net/nova/+bug/1337349/comments/6

> > All of this is very confusing and need to be sorted out before building
> > anything on top of it. Especially since "model" and "system-id" are
> > supposed to derive from VPD IIUC.
> > 
> > I guess that we should first decide what we really want to expose
> > in "system-id" and "model": what we have now ? the same as in
> > "host-serial" and "host-model", ie. user configurable ? Must we
> > stay compatible with existing setups ? In any case, I'm afraid that
> > we have to diverge from PAPR somehow, since it obviously doesn't
> > care about the security concerns that motivated recent changes
> > for "host-serial" and "host-model"...  
> 
> We absolutely should not expose the real host information without the
> user explicitly requesting it.
> 

I agree, but it seems that we may need something in between: an ID that
uniquely identifies a given system but that doesn't reveal security
sensitive information.

Looking again at PAPR, nothing much is said on the format of "system-id"
or VPD SN. Only "model" is expected to follow some formatting rules:
<vendor>,xxxx-yyy.

For "system-id", this could be addressed pretty much like in OpenStack
with systemd's /etc/machine-id. For "model", I'm not sure...

> > > Since it's a POWER specific
> > > functionality, may 'ibm,get-vpd' export host information if the
> > > guest instance allows it? Or is it better return only the 'host-serial'
> > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > of host-serial and host-model values"?
> > >   
> > 
> > I've spent some time reading PAPR on this topic and I'm not sure that
> > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > initially (ie, obtain up-to-date host model and serial after migration).
> > 
> > Have you looked at RTAS "ibm,update-properties" ?
> > 
> > 	7.4.8 ibm,update-properties RTAS Call
> > 
> > 	This RTAS call is used to report updates to the properties changed
> > 	due to a massive platform reconfiguration such as when the partition
> > 	is migrated between machines.  
> 
> Yeah.. the thing here is that partition migration kind of means
> something different in PAPR than it does in the qemu world.  It uses a
> guest-aware model of migration (which frankly I think is broken
> verging on unworkable).  qemu and uses a guest-(mostly)-unaware
> migration model.
> 

Yeah...

> > This explicitly covers updates to "system-id" and "model". Maybe it is
> > time to do as Ben was suggesting a long time ago when "host-serial"
> > and "host-model" were introduced [1]:
> > 
> > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > 	> Please be aware that all of the above is bogus when you start
> > 	> thinking 
> > 	> about live migration.  
> > 
> > 	What's probably where we need to start thinking about implementing
> > 	migration according to PAPR :-)
> > 
> > 	IE. With pre and post-migration notifications to the guest including
> > 	device-tree updates.
> > 
> > 
> > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> >   
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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 <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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.

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 <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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.

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5387 bytes --]

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 <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 9822 bytes --]

On Mon, Apr 08, 2019 at 06:31:56PM +0200, Greg Kurz wrote:
> On Mon, 8 Apr 2019 14:21:50 +1000
> David Gibson <david@gibson.dropbear.id.au> 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" <maxiwell@linux.ibm.com> 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" <maxiwell@linux.ibm.com> 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) ?

I would say yes, but I can't really be bothered arguing it upstream.

> > >        serial_number
> > >        The serial number of the physical system in which the partition resides
> > > 
> > >        system_type
> > >        The  machine,type-model  of  the physical system in which the partition
> > >        resides
> > > 
> > > This is indeed what we get in a PowerVM LPAR running on a tuleta system:
> > > 
> > > [root@furax1 ~]# head -3 /proc/ppc64/lparcfg 
> > > lparcfg 1.9
> > > serial_number=IBM,032116A9A
> > > system_type=IBM,8247-22L
> > > 
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/system-id)
> > > IBM,032116A9A
> > > [root@furax1 ~]# echo $(cat /proc/device-tree/model)
> > > IBM,8247-22L
> > > 
> > > But QEMU generates a hard coded "IBM pSeries (emulated by qemu)" model,
> > > which is clearly not PAPR compliant according to this requirement:
> > > 
> > > 	R1–12.2–13. There must be a property, “model”, under the root node
> > > 	in the format, “<vendor>,xxxx-yyy”, where <vendor> is replaced by
> > > 	one to five letters representing the stock symbol of the company
> > > 	(for example, for IBM: “IBM,xxxx-yyy”), and where xxxx-yyy is
> > > 	derived from the VPD TM field (see Table 160‚ “LoPAPR VPD Fields‚”
> > > 	on page 343) of the first or ‘marked’ processor enclosure.
> > > 
> > > And worse, it mixes "vm,uuid" which is QEMU specific concept to uniquely
> > > identify guests, with "system-id" which is about the host :-\  
> > 
> > Ugh, right.  So, it's been this way for years, so it's clearly not
> > breaking things in practice.  Given that, it might be best to leave
> > it, even though it violates PAPR technically.
> > 
> > Frankly, providing information about the *guest* virtual model and
> > "serial number" =~= UUID makes more sense than providing information
> > about the host that might be security sensitive.
> > 
> 
> IIUC, the rationale of exposing to the guest something that uniquely
> identifies the host is needed by some inventory or license validation
> softwares [1]. So I'd say we need both a guest UUID, that doesn't change
> during the VM lifetime, and host UUID that changes when migrating to
> another host.

Well, maybe.  We can supply the anonymized host identifier through
/etc/machine-id (already used by Openstack on x86), and I guess we can
present it to the guest via this get-vpd interface.  Heaven knows what
to do about caching / notifications, since in the qemu model (unlike
the PAPR / pHyp model) the guest is not aware of migrations.
> 
> The fact that "system-id" == "vm,uuid" in the current code didn't
> break anything would rather indicate that there are no inventory
> or license validation softwares users yet in the KVM+PAPR+linux
> ecosystem...

Well, maybe.

> [1] https://bugs.launchpad.net/nova/+bug/1337349/comments/6
> 
> > > All of this is very confusing and need to be sorted out before building
> > > anything on top of it. Especially since "model" and "system-id" are
> > > supposed to derive from VPD IIUC.
> > > 
> > > I guess that we should first decide what we really want to expose
> > > in "system-id" and "model": what we have now ? the same as in
> > > "host-serial" and "host-model", ie. user configurable ? Must we
> > > stay compatible with existing setups ? In any case, I'm afraid that
> > > we have to diverge from PAPR somehow, since it obviously doesn't
> > > care about the security concerns that motivated recent changes
> > > for "host-serial" and "host-model"...  
> > 
> > We absolutely should not expose the real host information without the
> > user explicitly requesting it.
> 
> I agree, but it seems that we may need something in between: an ID that
> uniquely identifies a given system but that doesn't reveal security
> sensitive information.
> 
> Looking again at PAPR, nothing much is said on the format of "system-id"
> or VPD SN. Only "model" is expected to follow some formatting rules:
> <vendor>,xxxx-yyy.
> 
> For "system-id", this could be addressed pretty much like in OpenStack
> with systemd's /etc/machine-id. For "model", I'm not sure...

Right, but do license systems need the model as well?  I can't see why
they would.

> > > > Since it's a POWER specific
> > > > functionality, may 'ibm,get-vpd' export host information if the
> > > > guest instance allows it? Or is it better return only the 'host-serial'
> > > > and 'host-model' content, like in the patch "spapr: Simplify handling
> > > > of host-serial and host-model values"?
> > > >   
> > > 
> > > I've spent some time reading PAPR on this topic and I'm not sure that
> > > "ibm,get-vpd" is the way to go for what you were trying to achieve
> > > initially (ie, obtain up-to-date host model and serial after migration).
> > > 
> > > Have you looked at RTAS "ibm,update-properties" ?
> > > 
> > > 	7.4.8 ibm,update-properties RTAS Call
> > > 
> > > 	This RTAS call is used to report updates to the properties changed
> > > 	due to a massive platform reconfiguration such as when the partition
> > > 	is migrated between machines.  
> > 
> > Yeah.. the thing here is that partition migration kind of means
> > something different in PAPR than it does in the qemu world.  It uses a
> > guest-aware model of migration (which frankly I think is broken
> > verging on unworkable).  qemu and uses a guest-(mostly)-unaware
> > migration model.
> 
> Yeah...
> 
> > > This explicitly covers updates to "system-id" and "model". Maybe it is
> > > time to do as Ben was suggesting a long time ago when "host-serial"
> > > and "host-model" were introduced [1]:
> > > 
> > > 	On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:  
> > > 	> Please be aware that all of the above is bogus when you start
> > > 	> thinking 
> > > 	> about live migration.  
> > > 
> > > 	What's probably where we need to start thinking about implementing
> > > 	migration according to PAPR :-)
> > > 
> > > 	IE. With pre and post-migration notifications to the guest including
> > > 	device-tree updates.
> > > 
> > > 
> > > [1] https://patchwork.ozlabs.org/patch/367792/#813547
> > >   
> > 
> 



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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]