[PATCH 00/07] HVM firmware passthrough

[PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

This patch series introduces support of loading HVMLOADER extension modules
into a guest. These modules can contain firmware information that is used
by HVMLOADER to modify a guests virtual firmware at startup. These modules
are only used by HVMLOADER.

The HVM module framework is generic allowing it to load any number of 
modules irrespective of the contents. The domain building code in libxenguest
reads the modules and loads them into the new guest. The loading is done in
what will become the guests low RAM area just behind to load location for
HVMLOADER. A header structure is constructed which locates the module set
and this header's base GPA is passed to the HVMLOADER in the ECX:EDX 
registers. The early entry code in HVMLOADER fetches these values and
initializes module support.

Currently two types of firmware information are recognized and processed
in the HVMLOADER though this could be extended.
1. SMBIOS: The SMBIOS table building code will attempt to retrieve a
predefined set of tables it allows to be overridden by passed through
tables. If a match is found the passed in table will be used. In addition
the SMBIOS code will also enumerate and process as many vendor defined
tables (in the range of types 128 - 255) as are passed in.
2. ACPI: Static and code (SSDTs) tables can be added to the set of ACPI
table built by HVMLOADER. The ACPI builder code will enumerate passed in
tables and add them at the end of the secondary table list.

There are 7 patches in the series:
01 - Add public HVM definitions header for firmware module passthrough support.
02 - Add module processing support in HVMLOADER.
03 - Fetch module set base address and initialize module support.
04 - Pass through support for SMBIOS.
05 - Pass through support for ACPI.
06 - Module file reading utility.
07 - Xen control tools support for loading the firmware modules.

Signed-off-by: Ross Philipson <ross.philipson@citrix.com>

Re: [PATCH 00/07] HVM firmware passthrough

[Keir Fraser]

On 19/03/2012 22:04, "Ross Philipson" <Ross.Philipson@citrix.com> wrote:

> This patch series introduces support of loading HVMLOADER extension modules
> into a guest. These modules can contain firmware information that is used
> by HVMLOADER to modify a guests virtual firmware at startup. These modules
> are only used by HVMLOADER.

There has been agreement, and work mainly by Tim, to get rid of our existing
hvm_info_table structure. This is similar in some ways, communicating
between builder and hvmloader via guest memory, but on major steroids.

Actually it looks like this also whacks some parameters through xenstore as
well (which is the newer way that Tim had been adding). So is this some
kludgy combination of old and new?

Would like feedback at least from Tim.

 -- Keir

> The HVM module framework is generic allowing it to load any number of
> modules irrespective of the contents. The domain building code in libxenguest
> reads the modules and loads them into the new guest. The loading is done in
> what will become the guests low RAM area just behind to load location for
> HVMLOADER. A header structure is constructed which locates the module set
> and this header's base GPA is passed to the HVMLOADER in the ECX:EDX
> registers. The early entry code in HVMLOADER fetches these values and
> initializes module support.
> 
> Currently two types of firmware information are recognized and processed
> in the HVMLOADER though this could be extended.
> 1. SMBIOS: The SMBIOS table building code will attempt to retrieve a
> predefined set of tables it allows to be overridden by passed through
> tables. If a match is found the passed in table will be used. In addition
> the SMBIOS code will also enumerate and process as many vendor defined
> tables (in the range of types 128 - 255) as are passed in.
> 2. ACPI: Static and code (SSDTs) tables can be added to the set of ACPI
> table built by HVMLOADER. The ACPI builder code will enumerate passed in
> tables and add them at the end of the secondary table list.
> 
> There are 7 patches in the series:
> 01 - Add public HVM definitions header for firmware module passthrough
> support.
> 02 - Add module processing support in HVMLOADER.
> 03 - Fetch module set base address and initialize module support.
> 04 - Pass through support for SMBIOS.
> 05 - Pass through support for ACPI.
> 06 - Module file reading utility.
> 07 - Xen control tools support for loading the firmware modules.
> 
> Signed-off-by: Ross Philipson <ross.philipson@citrix.com>
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

Well I am keen to hear what other have to say but here are some of my thoughts. I built the framework to be really generic so you could pass just about anything in using these modules. It doesn't really rely on xenstore at all but I guess I was under the impression that that is how folks wanted simple information to be passed to a nascent guest. Things like bios strings and certainly the bits we currently pass in via the hvm_info_page could go through xenstore. But of course the types of things I am passing here really have no place in xenstore. So we could use the dual approach (which seems fine to me) or we could pass everything in as another module type directly into guest memory.

If we use xenstore I think we should wrap it up a bit. In the process of doing this patch set I wrote a hvm assist library that builds the modules (which you may also want upstream). This might be a good place to put an API to sort of codify what can be set for hvmloader's consumption.

And FYI I am more than willing to work on these things and help replace the info page.

Thanks
Ross

> -----Original Message-----
> From: Keir Fraser [mailto:keir.xen@gmail.com]
> Sent: Monday, March 19, 2012 7:45 PM
> To: Ross Philipson; xen-devel@lists.xensource.com
> Cc: Tim (Xen.org); Ian Campbell
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> On 19/03/2012 22:04, "Ross Philipson" <Ross.Philipson@citrix.com> wrote:
> 
> > This patch series introduces support of loading HVMLOADER extension
> > modules into a guest. These modules can contain firmware information
> > that is used by HVMLOADER to modify a guests virtual firmware at
> > startup. These modules are only used by HVMLOADER.
> 
> There has been agreement, and work mainly by Tim, to get rid of our
> existing hvm_info_table structure. This is similar in some ways,
> communicating between builder and hvmloader via guest memory, but on
> major steroids.
> 
> Actually it looks like this also whacks some parameters through xenstore
> as well (which is the newer way that Tim had been adding). So is this
> some kludgy combination of old and new?
> 
> Would like feedback at least from Tim.
> 
>  -- Keir
> 
> > The HVM module framework is generic allowing it to load any number of
> > modules irrespective of the contents. The domain building code in
> > libxenguest reads the modules and loads them into the new guest. The
> > loading is done in what will become the guests low RAM area just
> > behind to load location for HVMLOADER. A header structure is
> > constructed which locates the module set and this header's base GPA is
> > passed to the HVMLOADER in the ECX:EDX registers. The early entry code
> > in HVMLOADER fetches these values and initializes module support.
> >
> > Currently two types of firmware information are recognized and
> > processed in the HVMLOADER though this could be extended.
> > 1. SMBIOS: The SMBIOS table building code will attempt to retrieve a
> > predefined set of tables it allows to be overridden by passed through
> > tables. If a match is found the passed in table will be used. In
> > addition the SMBIOS code will also enumerate and process as many
> > vendor defined tables (in the range of types 128 - 255) as are passed
> in.
> > 2. ACPI: Static and code (SSDTs) tables can be added to the set of
> > ACPI table built by HVMLOADER. The ACPI builder code will enumerate
> > passed in tables and add them at the end of the secondary table list.
> >
> > There are 7 patches in the series:
> > 01 - Add public HVM definitions header for firmware module passthrough
> > support.
> > 02 - Add module processing support in HVMLOADER.
> > 03 - Fetch module set base address and initialize module support.
> > 04 - Pass through support for SMBIOS.
> > 05 - Pass through support for ACPI.
> > 06 - Module file reading utility.
> > 07 - Xen control tools support for loading the firmware modules.
> >
> > Signed-off-by: Ross Philipson <ross.philipson@citrix.com>
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

Re: [PATCH 00/07] HVM firmware passthrough

[Tim Deegan]

At 23:44 +0000 on 19 Mar (1332200698), Keir Fraser wrote:
> On 19/03/2012 22:04, "Ross Philipson" <Ross.Philipson@citrix.com> wrote:
> 
> > This patch series introduces support of loading HVMLOADER extension modules
> > into a guest. These modules can contain firmware information that is used
> > by HVMLOADER to modify a guests virtual firmware at startup. These modules
> > are only used by HVMLOADER.
> 
> There has been agreement, and work mainly by Tim, to get rid of our existing
> hvm_info_table structure. This is similar in some ways, communicating
> between builder and hvmloader via guest memory, but on major steroids.
> 
> Actually it looks like this also whacks some parameters through xenstore as
> well (which is the newer way that Tim had been adding). So is this some
> kludgy combination of old and new?
> 
> Would like feedback at least from Tim.

My impression from the earlier discussions is that we're pasing largish
blobs of binary BIOS goop around, which aren't suitable to go into
xenstore.  Dropping them in memory where HVMloader can pick them up
seems reasonable.

All the control-path stuff - what the blobs are, where they are &c,
should go through Xenstore, though.

I'll take a look at the patches in detail later in the week. 

Tim.

> > The HVM module framework is generic allowing it to load any number of
> > modules irrespective of the contents. The domain building code in libxenguest
> > reads the modules and loads them into the new guest. The loading is done in
> > what will become the guests low RAM area just behind to load location for
> > HVMLOADER. A header structure is constructed which locates the module set
> > and this header's base GPA is passed to the HVMLOADER in the ECX:EDX
> > registers. The early entry code in HVMLOADER fetches these values and
> > initializes module support.
> > 
> > Currently two types of firmware information are recognized and processed
> > in the HVMLOADER though this could be extended.
> > 1. SMBIOS: The SMBIOS table building code will attempt to retrieve a
> > predefined set of tables it allows to be overridden by passed through
> > tables. If a match is found the passed in table will be used. In addition
> > the SMBIOS code will also enumerate and process as many vendor defined
> > tables (in the range of types 128 - 255) as are passed in.
> > 2. ACPI: Static and code (SSDTs) tables can be added to the set of ACPI
> > table built by HVMLOADER. The ACPI builder code will enumerate passed in
> > tables and add them at the end of the secondary table list.
> > 
> > There are 7 patches in the series:
> > 01 - Add public HVM definitions header for firmware module passthrough
> > support.
> > 02 - Add module processing support in HVMLOADER.
> > 03 - Fetch module set base address and initialize module support.
> > 04 - Pass through support for SMBIOS.
> > 05 - Pass through support for ACPI.
> > 06 - Module file reading utility.
> > 07 - Xen control tools support for loading the firmware modules.
> > 
> > Signed-off-by: Ross Philipson <ross.philipson@citrix.com>
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
> 

Re: [PATCH 00/07] HVM firmware passthrough

[Tim Deegan]

Hi, 

At 09:24 +0000 on 20 Mar (1332235452), Tim Deegan wrote:
> My impression from the earlier discussions is that we're pasing largish
> blobs of binary BIOS goop around, which aren't suitable to go into
> xenstore.  Dropping them in memory where HVMloader can pick them up
> seems reasonable.
> 
> All the control-path stuff - what the blobs are, where they are &c,
> should go through Xenstore, though.

So having looked at the code, I think the module system is really
overkill - AIUI all the things you're talking about passing through are
ACPI tables, which have their own length fields internally.  So all
you'd need to do is have a type code and an address in xenstore
somewhere, the same way we pass a type code and a string for the other
BIOS customizations.

Cheers,

Tim.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

 -----Original Message-----
> From: Tim Deegan [mailto:tim@xen.org]
> Sent: Thursday, March 22, 2012 7:26 AM
> To: Keir Fraser
> Cc: xen-devel@lists.xensource.com; Ian Campbell; Ross Philipson
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> Hi,
> 
> At 09:24 +0000 on 20 Mar (1332235452), Tim Deegan wrote:
> > My impression from the earlier discussions is that we're pasing
> > largish blobs of binary BIOS goop around, which aren't suitable to go
> > into xenstore.  Dropping them in memory where HVMloader can pick them
> > up seems reasonable.
> >
> > All the control-path stuff - what the blobs are, where they are &c,
> > should go through Xenstore, though.
> 
> So having looked at the code, I think the module system is really
> overkill - AIUI all the things you're talking about passing through are
> ACPI tables, which have their own length fields internally. So all

Ok well fair enough. I can go back and do it again, making something smaller. For the record, I am passing in more than just ACPI tables; I am passing in parts of the SMBIOS tables. Each of the SMBIOS types does not actually report its length but rather you have to parse them for the double terminator after the string section. I guess it seems a shame to have to do that parse logic in two places.

> you'd need to do is have a type code and an address in xenstore
> somewhere, the same way we pass a type code and a string for the other
> BIOS customizations.
> 

So I am not exactly sure how to proceed here since libxc seems the correct place to load the individual blobs (ACPI tables, SMBIOS junk) but libxc seems too low level to be writing values to xenstore (and it does not do that at present). Would it be better to have a peer API to xc_hvm_build() that write the chunks and returns the addresses where it loaded them? Or should it be totally moved out of libxc? Advice would be appreciated...

Finally, I had made this a generic framework because I thought it could be extended in the future to pass in other types of firmware-ish blobs at runtime. It also handles the case where the passing of one set of tables/blobs is not tied to passing another set. E.g. in our work we pass in some static ACPI tables to satisfy one feature and construct/pass-in an SSDT to satisfy another unrelated feature.

> Cheers,
> 
> Tim.

Thanks
Ross

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

Seems like this thread slipped through the cracks, sorry about that.
Please do give us a prod (e.g. a ping message) after a few days/a week
when this happens.

On Thu, 2012-03-22 at 14:03 +0000, Ross Philipson wrote:
> -----Original Message-----
> > From: Tim Deegan [mailto:tim@xen.org]
> > Sent: Thursday, March 22, 2012 7:26 AM
> > To: Keir Fraser
> > Cc: xen-devel@lists.xensource.com; Ian Campbell; Ross Philipson
> > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > 
> > Hi,
> > 
> > At 09:24 +0000 on 20 Mar (1332235452), Tim Deegan wrote:
> > > My impression from the earlier discussions is that we're pasing
> > > largish blobs of binary BIOS goop around, which aren't suitable to go
> > > into xenstore.  Dropping them in memory where HVMloader can pick them
> > > up seems reasonable.
> > >
> > > All the control-path stuff - what the blobs are, where they are &c,
> > > should go through Xenstore, though.
> > 
> > So having looked at the code, I think the module system is really
> > overkill - AIUI all the things you're talking about passing through are
> > ACPI tables, which have their own length fields internally. So all
> 
> Ok well fair enough. I can go back and do it again, making something
> smaller. For the record, I am passing in more than just ACPI tables; I
> am passing in parts of the SMBIOS tables. Each of the SMBIOS types
> does not actually report its length but rather you have to parse them
> for the double terminator after the string section. I guess it seems a
> shame to have to do that parse logic in two places.

I think Tim was suggesting that in xs this looks something like:
	.../hvmloader/acpi/address 
	(no .../acpi/length length? encoded in tables)
	.../hvmloader/smbios/address
	.../hvmloader/smbios/length
	.../hvmloader/some-other/{address,length?}...

with the data at the referenced addresses.

If it's convenient (or even just for consistency) there's no reason IMHO
not to include the length even where the length is part of the data.
Similarly you can add checksums etc if those are useful.

> > you'd need to do is have a type code and an address in xenstore
> > somewhere, the same way we pass a type code and a string for the other
> > BIOS customizations.
> > 
> 
> So I am not exactly sure how to proceed here since libxc seems the
> correct place to load the individual blobs (ACPI tables, SMBIOS junk)
> but libxc seems too low level to be writing values to xenstore (and it
> does not do that at present). Would it be better to have a peer API to
> xc_hvm_build() that write the chunks and returns the addresses where
> it loaded them? Or should it be totally moved out of libxc? Advice
> would be appreciated...

The layering relationship between libxc and libxs here is a bit of a
pain, but lets not try and solve that now.

I think you can just add a guest_address field to your struct
xc_hvm_module as an output field which the caller would then push into
xenstore along with the (potentially updated) length field.

Given the above XS layout then I think where you have a list of modules in
xc_hvm_build_args you can just have "struct xc_hvm_module acpi".
Similarly for smbios and whatever new table types come up in the future.
I don't think having each module type explicitly here matters since each
new table type needs a bunch of support code in at least hvmloader
anyway so adding a few lines to libxc to expose it at the same time is
fine.

> Finally, I had made this a generic framework because I thought it
> could be extended in the future to pass in other types of firmware-ish
> blobs at runtime. It also handles the case where the passing of one
> set of tables/blobs is not tied to passing another set. E.g. in our
> work we pass in some static ACPI tables to satisfy one feature and
> construct/pass-in an SSDT to satisfy another unrelated feature.

I think it is ok to have it be up to the caller to glom those together
into a single "ACPI" blob which is processed by hvmloader into the right
places. Or is there a problem with that which hasn't occurred to me?
(Likewise in the SMBIOS case)

If it is a problem then
	.../acpi/0/...
	.../acpi/1/...
(or s/0/SSDT0/ and s/1/SSDT1/ etc) works for the XS side of things. Not
sure about the libxc level, I'll worry about it when you tell me what
I've missed which makes it necessary ;-)


Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Wednesday, April 04, 2012 5:30 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> Seems like this thread slipped through the cracks, sorry about that.
> Please do give us a prod (e.g. a ping message) after a few days/a week
> when this happens.

Ok, thanks, will do.

> 
> On Thu, 2012-03-22 at 14:03 +0000, Ross Philipson wrote:
> > -----Original Message-----
> > > From: Tim Deegan [mailto:tim@xen.org]
> > > Sent: Thursday, March 22, 2012 7:26 AM
> > > To: Keir Fraser
> > > Cc: xen-devel@lists.xensource.com; Ian Campbell; Ross Philipson
> > > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > >
> > > Hi,
> > >
> > > At 09:24 +0000 on 20 Mar (1332235452), Tim Deegan wrote:
> > > > My impression from the earlier discussions is that we're pasing
> > > > largish blobs of binary BIOS goop around, which aren't suitable to
> > > > go into xenstore.  Dropping them in memory where HVMloader can
> > > > pick them up seems reasonable.
> > > >
> > > > All the control-path stuff - what the blobs are, where they are
> > > > &c, should go through Xenstore, though.
> > >
> > > So having looked at the code, I think the module system is really
> > > overkill - AIUI all the things you're talking about passing through
> > > are ACPI tables, which have their own length fields internally. So
> > > all
> >
> > Ok well fair enough. I can go back and do it again, making something
> > smaller. For the record, I am passing in more than just ACPI tables; I
> > am passing in parts of the SMBIOS tables. Each of the SMBIOS types
> > does not actually report its length but rather you have to parse them
> > for the double terminator after the string section. I guess it seems a
> > shame to have to do that parse logic in two places.
> 
> I think Tim was suggesting that in xs this looks something like:
> 	.../hvmloader/acpi/address
> 	(no .../acpi/length length? encoded in tables)
> 	.../hvmloader/smbios/address
> 	.../hvmloader/smbios/length
> 	.../hvmloader/some-other/{address,length?}...
> 
> with the data at the referenced addresses.
> 
> If it's convenient (or even just for consistency) there's no reason IMHO
> not to include the length even where the length is part of the data.
> Similarly you can add checksums etc if those are useful.
> 

Well actually it does not really do any good to have the length in xenstore (at least for the types we are currently talking about) other than knowing the overall extent of the blob. The problem is that we are talking about multiple tables chained together. For ACPI this is not a problem because the table length is specified in the table header. For SMBIOS (where there will almost certainly be multiple tables) the length of each table is not specified and so the set would have to be re-parsed in hvmloader to find each individual table. That was the motivation for a separate table header we define. I still think some simple entry delimiter structure with some small amount of information about each item is a good idea.

But in general I don't have a problem with using xenstore for this sort of thing so I am fine with the basic concept.

> > > you'd need to do is have a type code and an address in xenstore
> > > somewhere, the same way we pass a type code and a string for the
> > > other BIOS customizations.
> > >
> >
> > So I am not exactly sure how to proceed here since libxc seems the
> > correct place to load the individual blobs (ACPI tables, SMBIOS junk)
> > but libxc seems too low level to be writing values to xenstore (and it
> > does not do that at present). Would it be better to have a peer API to
> > xc_hvm_build() that write the chunks and returns the addresses where
> > it loaded them? Or should it be totally moved out of libxc? Advice
> > would be appreciated...
> 
> The layering relationship between libxc and libxs here is a bit of a
> pain, but lets not try and solve that now.
> 
> I think you can just add a guest_address field to your struct
> xc_hvm_module as an output field which the caller would then push into
> xenstore along with the (potentially updated) length field.
> 
> Given the above XS layout then I think where you have a list of modules
> in xc_hvm_build_args you can just have "struct xc_hvm_module acpi".
> Similarly for smbios and whatever new table types come up in the future.
> I don't think having each module type explicitly here matters since each
> new table type needs a bunch of support code in at least hvmloader
> anyway so adding a few lines to libxc to expose it at the same time is
> fine.

That seems like a reasonable way to do it. Especially if (wrt below) we have the caller glom the like bits together.

> 
> > Finally, I had made this a generic framework because I thought it
> > could be extended in the future to pass in other types of firmware-ish
> > blobs at runtime. It also handles the case where the passing of one
> > set of tables/blobs is not tied to passing another set. E.g. in our
> > work we pass in some static ACPI tables to satisfy one feature and
> > construct/pass-in an SSDT to satisfy another unrelated feature.
> 
> I think it is ok to have it be up to the caller to glom those together
> into a single "ACPI" blob which is processed by hvmloader into the right
> places. Or is there a problem with that which hasn't occurred to me?
> (Likewise in the SMBIOS case)
> 
> If it is a problem then
> 	.../acpi/0/...
> 	.../acpi/1/...
> (or s/0/SSDT0/ and s/1/SSDT1/ etc) works for the XS side of things. Not
> sure about the libxc level, I'll worry about it when you tell me what
> I've missed which makes it necessary ;-)

Well that approach was mostly for flexibility (as in the scenario I pointed out). The only other issue might be the measuring of components by a domain builder but I don't think that that prevents glomming. Presumably the glomming code is part of the overall domain builder code so a measure-then-glom operation can happen. And doing the merge in the tools makes the code in hvmloader simpler so I am good with that.

Given all that, the only real issue I see at the moment is how to deal with multiple entries within a blob that don't readily specify their own lengths. I am in favor of a delimiting struct with possibly a terminating form (like a flag). Then all we put in xenstore is the gpe for each type.

Finally I wanted to bring up again the idea of another helper component (lib maybe) that can be used to build and glom (I like that word) modules. Note that in many cases the passed in firmware is going to be pulled from the host firmware. I already have bunches of codes that do that. Perhaps I should start an RFC thread for that as a separate task? Thoughts on this?

Thanks
Ross

> 
> 
> Ian.
> 

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Wed, 2012-04-04 at 20:25 +0100, Ross Philipson wrote:
> > If it's convenient (or even just for consistency) there's no reason IMHO
> > not to include the length even where the length is part of the data.
> > Similarly you can add checksums etc if those are useful.
> > 
> 
> Well actually it does not really do any good to have the length in
> xenstore (at least for the types we are currently talking about) other
> than knowing the overall extent of the blob. The problem is that we
> are talking about multiple tables chained together. For ACPI this is
> not a problem because the table length is specified in the table
> header. For SMBIOS (where there will almost certainly be multiple
> tables) the length of each table is not specified and so the set would
> have to be re-parsed in hvmloader to find each individual table.

I see. So you need to be able to find the individual tables so that
smbios_type_<N>_init can check for an overriding table <N> in the
passed-through tables, it seems reasonable to try and avoid needing to
parse a big lump of tables each time to see if the one you are
interested in is there.

I think this can work by having .../smbios/<N>/{address,etc} in
XenStore. You could also have .../smbios/OEM/{...} for the stuff for
smbios_type_vendor_oem_init, which I think could easily just be a single
lump?

I think you don't need the same thing for the ACPI side since there you
just provide secondary tables?

>  That was the motivation for a separate table header we define. I
> still think some simple entry delimiter structure with some small
> amount of information about each item is a good idea.

I think the content of .../smbios/<N>/* serves the same purpose as a
delimiter?

> But in general I don't have a problem with using xenstore for this
> sort of thing so I am fine with the basic concept.
> 
> > > > you'd need to do is have a type code and an address in xenstore
> > > > somewhere, the same way we pass a type code and a string for the
> > > > other BIOS customizations.
> > > >
> > >
> > > So I am not exactly sure how to proceed here since libxc seems the
> > > correct place to load the individual blobs (ACPI tables, SMBIOS junk)
> > > but libxc seems too low level to be writing values to xenstore (and it
> > > does not do that at present). Would it be better to have a peer API to
> > > xc_hvm_build() that write the chunks and returns the addresses where
> > > it loaded them? Or should it be totally moved out of libxc? Advice
> > > would be appreciated...
> > 
> > The layering relationship between libxc and libxs here is a bit of a
> > pain, but lets not try and solve that now.
> > 
> > I think you can just add a guest_address field to your struct
> > xc_hvm_module as an output field which the caller would then push into
> > xenstore along with the (potentially updated) length field.
> > 
> > Given the above XS layout then I think where you have a list of modules
> > in xc_hvm_build_args you can just have "struct xc_hvm_module acpi".
> > Similarly for smbios and whatever new table types come up in the future.
> > I don't think having each module type explicitly here matters since each
> > new table type needs a bunch of support code in at least hvmloader
> > anyway so adding a few lines to libxc to expose it at the same time is
> > fine.
> 
> That seems like a reasonable way to do it. Especially if (wrt below)
> we have the caller glom the like bits together.
> 
> > 
> > > Finally, I had made this a generic framework because I thought it
> > > could be extended in the future to pass in other types of firmware-ish
> > > blobs at runtime. It also handles the case where the passing of one
> > > set of tables/blobs is not tied to passing another set. E.g. in our
> > > work we pass in some static ACPI tables to satisfy one feature and
> > > construct/pass-in an SSDT to satisfy another unrelated feature.
> > 
> > I think it is ok to have it be up to the caller to glom those together
> > into a single "ACPI" blob which is processed by hvmloader into the right
> > places. Or is there a problem with that which hasn't occurred to me?
> > (Likewise in the SMBIOS case)
> > 
> > If it is a problem then
> > 	.../acpi/0/...
> > 	.../acpi/1/...
> > (or s/0/SSDT0/ and s/1/SSDT1/ etc) works for the XS side of things. Not
> > sure about the libxc level, I'll worry about it when you tell me what
> > I've missed which makes it necessary ;-)
> 
> Well that approach was mostly for flexibility (as in the scenario I
> pointed out). The only other issue might be the measuring of
> components by a domain builder but I don't think that that prevents
> glomming. Presumably the glomming code is part of the overall domain
> builder code so a measure-then-glom operation can happen. And doing
> the merge in the tools makes the code in hvmloader simpler so I am
> good with that.

You've convinced me that having the SMBIOS tables each be presented
separately makes sense.

At the libxc layer you could have "struct xc_hvm_module *smbios". I
expect in the simple case this would just point into an array on the
callers stack but a caller could also do something more dynamic if they
wanted to. If you think it would be useful then a linked-list thing here
would also work.

> Given all that, the only real issue I see at the moment is how to deal
> with multiple entries within a blob that don't readily specify their
> own lengths. I am in favor of a delimiting struct with possibly a
> terminating form (like a flag). Then all we put in xenstore is the gpe
> for each type.
> 
> Finally I wanted to bring up again the idea of another helper
> component (lib maybe) that can be used to build and glom (I like that
> word) modules. Note that in many cases the passed in firmware is going
> to be pulled from the host firmware. I already have bunches of codes
> that do that. Perhaps I should start an RFC thread for that as a
> separate task? Thoughts on this?

You mean some sort of libacpi / libsmbios type things, helper routines
for parsing and manipulating tables etc? (such a thing doesn't already
exist somewhere?)

I suspect your usecases are the ones which would make most extensive use
of such a thing but I also assume that at least some of it would be
useful to any caller which was passing in these tables. If you want to
maintain it within the Xen tree then that works for me.

Ian.

> 
> Thanks
> Ross
> 
> > 
> > 
> > Ian.
> > 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Thursday, April 05, 2012 6:08 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> On Wed, 2012-04-04 at 20:25 +0100, Ross Philipson wrote:
> > > If it's convenient (or even just for consistency) there's no reason
> > > IMHO not to include the length even where the length is part of the
> data.
> > > Similarly you can add checksums etc if those are useful.
> > >
> >
> > Well actually it does not really do any good to have the length in
> > xenstore (at least for the types we are currently talking about) other
> > than knowing the overall extent of the blob. The problem is that we
> > are talking about multiple tables chained together. For ACPI this is
> > not a problem because the table length is specified in the table
> > header. For SMBIOS (where there will almost certainly be multiple
> > tables) the length of each table is not specified and so the set would
> > have to be re-parsed in hvmloader to find each individual table.
> 
> I see. So you need to be able to find the individual tables so that
> smbios_type_<N>_init can check for an overriding table <N> in the
> passed-through tables, it seems reasonable to try and avoid needing to
> parse a big lump of tables each time to see if the one you are
> interested in is there.
> 
> I think this can work by having .../smbios/<N>/{address,etc} in
> XenStore. You could also have .../smbios/OEM/{...} for the stuff for
> smbios_type_vendor_oem_init, which I think could easily just be a single
> lump?
> 
> I think you don't need the same thing for the ACPI side since there you
> just provide secondary tables?

There could be quite a few SMBIOS tables being passed in. On the order of 20 - 30 of them and thet are pretty small. It seems a bit odd to break it up like this for lots of little chunks. There could be more than one ACPI table also (multiple SSDTs and/or other static tables). Also the OEM SMBIOS tables are all discrete and they could not go in as a single lump.

This approach will certainly work but I am still not sure about it being the best way. 

> 
> >  That was the motivation for a separate table header we define. I
> > still think some simple entry delimiter structure with some small
> > amount of information about each item is a good idea.
> 
> I think the content of .../smbios/<N>/* serves the same purpose as a
> delimiter?

Yes if we use this approach.

> 
> > But in general I don't have a problem with using xenstore for this
> > sort of thing so I am fine with the basic concept.
> >
> > > > > you'd need to do is have a type code and an address in xenstore
> > > > > somewhere, the same way we pass a type code and a string for the
> > > > > other BIOS customizations.
> > > > >
> > > >
> > > > So I am not exactly sure how to proceed here since libxc seems the
> > > > correct place to load the individual blobs (ACPI tables, SMBIOS
> > > > junk) but libxc seems too low level to be writing values to
> > > > xenstore (and it does not do that at present). Would it be better
> > > > to have a peer API to
> > > > xc_hvm_build() that write the chunks and returns the addresses
> > > > where it loaded them? Or should it be totally moved out of libxc?
> > > > Advice would be appreciated...
> > >
> > > The layering relationship between libxc and libxs here is a bit of a
> > > pain, but lets not try and solve that now.
> > >
> > > I think you can just add a guest_address field to your struct
> > > xc_hvm_module as an output field which the caller would then push
> > > into xenstore along with the (potentially updated) length field.
> > >
> > > Given the above XS layout then I think where you have a list of
> > > modules in xc_hvm_build_args you can just have "struct xc_hvm_module
> acpi".
> > > Similarly for smbios and whatever new table types come up in the
> future.
> > > I don't think having each module type explicitly here matters since
> > > each new table type needs a bunch of support code in at least
> > > hvmloader anyway so adding a few lines to libxc to expose it at the
> > > same time is fine.
> >
> > That seems like a reasonable way to do it. Especially if (wrt below)
> > we have the caller glom the like bits together.
> >
> > >
> > > > Finally, I had made this a generic framework because I thought it
> > > > could be extended in the future to pass in other types of
> > > > firmware-ish blobs at runtime. It also handles the case where the
> > > > passing of one set of tables/blobs is not tied to passing another
> > > > set. E.g. in our work we pass in some static ACPI tables to
> > > > satisfy one feature and construct/pass-in an SSDT to satisfy
> another unrelated feature.
> > >
> > > I think it is ok to have it be up to the caller to glom those
> > > together into a single "ACPI" blob which is processed by hvmloader
> > > into the right places. Or is there a problem with that which hasn't
> occurred to me?
> > > (Likewise in the SMBIOS case)
> > >
> > > If it is a problem then
> > > 	.../acpi/0/...
> > > 	.../acpi/1/...
> > > (or s/0/SSDT0/ and s/1/SSDT1/ etc) works for the XS side of things.
> > > Not sure about the libxc level, I'll worry about it when you tell me
> > > what I've missed which makes it necessary ;-)
> >
> > Well that approach was mostly for flexibility (as in the scenario I
> > pointed out). The only other issue might be the measuring of
> > components by a domain builder but I don't think that that prevents
> > glomming. Presumably the glomming code is part of the overall domain
> > builder code so a measure-then-glom operation can happen. And doing
> > the merge in the tools makes the code in hvmloader simpler so I am
> > good with that.
> 
> You've convinced me that having the SMBIOS tables each be presented
> separately makes sense.
> 
> At the libxc layer you could have "struct xc_hvm_module *smbios". I
> expect in the simple case this would just point into an array on the
> callers stack but a caller could also do something more dynamic if they
> wanted to. If you think it would be useful then a linked-list thing here
> would also work.

Again assuming the xenstore approach, I guess it would probably have to be a linked list because there could be a fair number of them (mainly SMBIOS).

> 
> > Given all that, the only real issue I see at the moment is how to deal
> > with multiple entries within a blob that don't readily specify their
> > own lengths. I am in favor of a delimiting struct with possibly a
> > terminating form (like a flag). Then all we put in xenstore is the gpe
> > for each type.
> >
> > Finally I wanted to bring up again the idea of another helper
> > component (lib maybe) that can be used to build and glom (I like that
> > word) modules. Note that in many cases the passed in firmware is going
> > to be pulled from the host firmware. I already have bunches of codes
> > that do that. Perhaps I should start an RFC thread for that as a
> > separate task? Thoughts on this?
> 
> You mean some sort of libacpi / libsmbios type things, helper routines
> for parsing and manipulating tables etc? (such a thing doesn't already
> exist somewhere?)
> 
> I suspect your usecases are the ones which would make most extensive use
> of such a thing but I also assume that at least some of it would be
> useful to any caller which was passing in these tables. If you want to
> maintain it within the Xen tree then that works for me.

Yea along those lines. I was thinking of maybe a libxenhvm that had utility routines for collecting the system firmware information that one wanted to pass to a guest. It could also provide an API that wrapped up the setting of the firmware values that hvmloader consumes including the bits that are on the shared info page you want to get rid of. The usefulness of this library is diminished if we go the all xenstore route above so I am going to shelf this until that is settled.

Oh and yea, there are tools to get this information (acpidump/dmidecode) but they unfortunately do not come in library form or necessarily give you the desired output. And in newer kernels ACPI/DMI is accessible through sysfs.

> 
> Ian.
> 
> >
> > Thanks
> > Ross
> >
> > >
> > >
> > > Ian.
> > >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Thu, 2012-04-05 at 21:06 +0100, Ross Philipson wrote:
> > I see. So you need to be able to find the individual tables so that
> > smbios_type_<N>_init can check for an overriding table <N> in the
> > passed-through tables, it seems reasonable to try and avoid needing to
> > parse a big lump of tables each time to see if the one you are
> > interested in is there.
> > 
> > I think this can work by having .../smbios/<N>/{address,etc} in
> > XenStore. You could also have .../smbios/OEM/{...} for the stuff for
> > smbios_type_vendor_oem_init, which I think could easily just be a single
> > lump?
> > 
> > I think you don't need the same thing for the ACPI side since there you
> > just provide secondary tables?
> 
> There could be quite a few SMBIOS tables being passed in. On the order
> of 20 - 30 of them and thet are pretty small. It seems a bit odd to
> break it up like this for lots of little chunks. There could be more
> than one ACPI table also (multiple SSDTs and/or other static tables).
> Also the OEM SMBIOS tables are all discrete and they could not go in
> as a single lump.

I'm not sure if there are arguments here both for and against having a
single smbios blob vs multiple?

> > At the libxc layer you could have "struct xc_hvm_module *smbios". I
> > expect in the simple case this would just point into an array on the
> > callers stack but a caller could also do something more dynamic if they
> > wanted to. If you think it would be useful then a linked-list thing here
> > would also work.
> 
> Again assuming the xenstore approach, I guess it would probably have
> to be a linked list because there could be a fair number of them
> (mainly SMBIOS).

This is userspace so an array of, say, 100 items isn't really much of a
problem. But if a link list works better for your use cases then that's
fine.

> > 
> > > Given all that, the only real issue I see at the moment is how to deal
> > > with multiple entries within a blob that don't readily specify their
> > > own lengths. I am in favor of a delimiting struct with possibly a
> > > terminating form (like a flag). Then all we put in xenstore is the gpe
> > > for each type.
> > >
> > > Finally I wanted to bring up again the idea of another helper
> > > component (lib maybe) that can be used to build and glom (I like that
> > > word) modules. Note that in many cases the passed in firmware is going
> > > to be pulled from the host firmware. I already have bunches of codes
> > > that do that. Perhaps I should start an RFC thread for that as a
> > > separate task? Thoughts on this?
> > 
> > You mean some sort of libacpi / libsmbios type things, helper routines
> > for parsing and manipulating tables etc? (such a thing doesn't already
> > exist somewhere?)
> > 
> > I suspect your usecases are the ones which would make most extensive use
> > of such a thing but I also assume that at least some of it would be
> > useful to any caller which was passing in these tables. If you want to
> > maintain it within the Xen tree then that works for me.
> 
> Yea along those lines. I was thinking of maybe a libxenhvm that had
> utility routines for collecting the system firmware information that
> one wanted to pass to a guest. It could also provide an API that
> wrapped up the setting of the firmware values that hvmloader consumes
> including the bits that are on the shared info page you want to get
> rid of. The usefulness of this library is diminished if we go the all
> xenstore route above so I am going to shelf this until that is
> settled.

OK.

> Oh and yea, there are tools to get this information
> (acpidump/dmidecode) but they unfortunately do not come in library
> form or necessarily give you the desired output. And in newer kernels
> ACPI/DMI is accessible through sysfs.

Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Tuesday, April 10, 2012 10:22 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> On Thu, 2012-04-05 at 21:06 +0100, Ross Philipson wrote:
> > > I see. So you need to be able to find the individual tables so that
> > > smbios_type_<N>_init can check for an overriding table <N> in the
> > > passed-through tables, it seems reasonable to try and avoid needing
> > > to parse a big lump of tables each time to see if the one you are
> > > interested in is there.
> > >
> > > I think this can work by having .../smbios/<N>/{address,etc} in
> > > XenStore. You could also have .../smbios/OEM/{...} for the stuff for
> > > smbios_type_vendor_oem_init, which I think could easily just be a
> > > single lump?
> > >
> > > I think you don't need the same thing for the ACPI side since there
> > > you just provide secondary tables?
> >
> > There could be quite a few SMBIOS tables being passed in. On the order
> > of 20 - 30 of them and thet are pretty small. It seems a bit odd to
> > break it up like this for lots of little chunks. There could be more
> > than one ACPI table also (multiple SSDTs and/or other static tables).
> > Also the OEM SMBIOS tables are all discrete and they could not go in
> > as a single lump.
> 
> I'm not sure if there are arguments here both for and against having a
> single smbios blob vs multiple?

Oh sorry. I am trying to answer 2 different things here. In summary what I would like to do is not make any distinction between vendor and predefined tables but rather send all SMBIOS tables in as a single lump with some sort of internal delimiter structs. If that is unacceptable then all the SMBIOS tables would be put in xenstore per what you said above (.../smbios/<N>/{address,etc}). It just seems odd to me to break up the (usually rather small and potentially numerous) SMBIOS tables into individual items passed in xenstore.

> 
> > > At the libxc layer you could have "struct xc_hvm_module *smbios". I
> > > expect in the simple case this would just point into an array on the
> > > callers stack but a caller could also do something more dynamic if
> > > they wanted to. If you think it would be useful then a linked-list
> > > thing here would also work.
> >
> > Again assuming the xenstore approach, I guess it would probably have
> > to be a linked list because there could be a fair number of them
> > (mainly SMBIOS).
> 
> This is userspace so an array of, say, 100 items isn't really much of a
> problem. But if a link list works better for your use cases then that's
> fine.
> 
> > >
> > > > Given all that, the only real issue I see at the moment is how to
> > > > deal with multiple entries within a blob that don't readily
> > > > specify their own lengths. I am in favor of a delimiting struct
> > > > with possibly a terminating form (like a flag). Then all we put in
> > > > xenstore is the gpe for each type.
> > > >
> > > > Finally I wanted to bring up again the idea of another helper
> > > > component (lib maybe) that can be used to build and glom (I like
> > > > that
> > > > word) modules. Note that in many cases the passed in firmware is
> > > > going to be pulled from the host firmware. I already have bunches
> > > > of codes that do that. Perhaps I should start an RFC thread for
> > > > that as a separate task? Thoughts on this?
> > >
> > > You mean some sort of libacpi / libsmbios type things, helper
> > > routines for parsing and manipulating tables etc? (such a thing
> > > doesn't already exist somewhere?)
> > >
> > > I suspect your usecases are the ones which would make most extensive
> > > use of such a thing but I also assume that at least some of it would
> > > be useful to any caller which was passing in these tables. If you
> > > want to maintain it within the Xen tree then that works for me.
> >
> > Yea along those lines. I was thinking of maybe a libxenhvm that had
> > utility routines for collecting the system firmware information that
> > one wanted to pass to a guest. It could also provide an API that
> > wrapped up the setting of the firmware values that hvmloader consumes
> > including the bits that are on the shared info page you want to get
> > rid of. The usefulness of this library is diminished if we go the all
> > xenstore route above so I am going to shelf this until that is
> > settled.
> 
> OK.
> 
> > Oh and yea, there are tools to get this information
> > (acpidump/dmidecode) but they unfortunately do not come in library
> > form or necessarily give you the desired output. And in newer kernels
> > ACPI/DMI is accessible through sysfs.
> 
> Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Tue, 2012-04-10 at 16:28 +0100, Ross Philipson wrote:
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: Tuesday, April 10, 2012 10:22 AM
> > To: Ross Philipson
> > Cc: xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > 
> > On Thu, 2012-04-05 at 21:06 +0100, Ross Philipson wrote:
> > > > I see. So you need to be able to find the individual tables so that
> > > > smbios_type_<N>_init can check for an overriding table <N> in the
> > > > passed-through tables, it seems reasonable to try and avoid needing
> > > > to parse a big lump of tables each time to see if the one you are
> > > > interested in is there.
> > > >
> > > > I think this can work by having .../smbios/<N>/{address,etc} in
> > > > XenStore. You could also have .../smbios/OEM/{...} for the stuff for
> > > > smbios_type_vendor_oem_init, which I think could easily just be a
> > > > single lump?
> > > >
> > > > I think you don't need the same thing for the ACPI side since there
> > > > you just provide secondary tables?
> > >
> > > There could be quite a few SMBIOS tables being passed in. On the order
> > > of 20 - 30 of them and thet are pretty small. It seems a bit odd to
> > > break it up like this for lots of little chunks. There could be more
> > > than one ACPI table also (multiple SSDTs and/or other static tables).
> > > Also the OEM SMBIOS tables are all discrete and they could not go in
> > > as a single lump.
> > 
> > I'm not sure if there are arguments here both for and against having a
> > single smbios blob vs multiple?
> 
> Oh sorry. I am trying to answer 2 different things here. In summary
> what I would like to do is not make any distinction between vendor and
> predefined tables but rather send all SMBIOS tables in as a single
> lump with some sort of internal delimiter structs. If that is
> unacceptable then all the SMBIOS tables would be put in xenstore per
> what you said above (.../smbios/<N>/{address,etc}). It just seems odd
> to me to break up the (usually rather small and potentially numerous)
> SMBIOS tables into individual items passed in xenstore.

What sort of delimiter were you thinking of? Perhaps something as simple
as:
	<length><blob><length><blob>
?

Or can you use the length field in the smbios entry header?

Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Tuesday, April 10, 2012 11:39 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com; Tim (Xen.org)
> Subject: RE: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> On Tue, 2012-04-10 at 16:28 +0100, Ross Philipson wrote:
> > > -----Original Message-----
> > > From: Ian Campbell
> > > Sent: Tuesday, April 10, 2012 10:22 AM
> > > To: Ross Philipson
> > > Cc: xen-devel@lists.xensource.com
> > > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > >
> > > On Thu, 2012-04-05 at 21:06 +0100, Ross Philipson wrote:
> > > > > I see. So you need to be able to find the individual tables so
> > > > > that smbios_type_<N>_init can check for an overriding table <N>
> > > > > in the passed-through tables, it seems reasonable to try and
> > > > > avoid needing to parse a big lump of tables each time to see if
> > > > > the one you are interested in is there.
> > > > >
> > > > > I think this can work by having .../smbios/<N>/{address,etc} in
> > > > > XenStore. You could also have .../smbios/OEM/{...} for the stuff
> > > > > for smbios_type_vendor_oem_init, which I think could easily just
> > > > > be a single lump?
> > > > >
> > > > > I think you don't need the same thing for the ACPI side since
> > > > > there you just provide secondary tables?
> > > >
> > > > There could be quite a few SMBIOS tables being passed in. On the
> > > > order of 20 - 30 of them and thet are pretty small. It seems a bit
> > > > odd to break it up like this for lots of little chunks. There
> > > > could be more than one ACPI table also (multiple SSDTs and/or
> other static tables).
> > > > Also the OEM SMBIOS tables are all discrete and they could not go
> > > > in as a single lump.
> > >
> > > I'm not sure if there are arguments here both for and against having
> > > a single smbios blob vs multiple?
> >
> > Oh sorry. I am trying to answer 2 different things here. In summary
> > what I would like to do is not make any distinction between vendor and
> > predefined tables but rather send all SMBIOS tables in as a single
> > lump with some sort of internal delimiter structs. If that is
> > unacceptable then all the SMBIOS tables would be put in xenstore per
> > what you said above (.../smbios/<N>/{address,etc}). It just seems odd
> > to me to break up the (usually rather small and potentially numerous)
> > SMBIOS tables into individual items passed in xenstore.
> 
> What sort of delimiter were you thinking of? Perhaps something as simple
> as:
> 	<length><blob><length><blob>

Yea, that would suffice.

> ?
> 
> Or can you use the length field in the smbios entry header?
> 
> Ian.
> 

Not really. I think part of the problem here is my mixing of terminology. For SMBIOS bits I am pulling apart the overall SMBIOS table and just grabbing a desired subset of the SMBIOS structures. The individual structures are the entities that do not have an overall length field so I want to stick them back together as one lump with a length delimiter - what you described above is perfectly sufficient.

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Tue, 2012-04-10 at 20:04 +0100, Ross Philipson wrote:
> Not really. I think part of the problem here is my mixing of
> terminology. For SMBIOS bits I am pulling apart the overall SMBIOS
> table and just grabbing a desired subset of the SMBIOS structures. The
> individual structures are the entities that do not have an overall
> length field

So, to use the terminology of tools/firmware/hvmloader/smbios_types.h,
you have entities which are subsets of a structure and so do not start
with a "struct smbios_structure_header"?

Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Ian Campbell
> Sent: Wednesday, April 11, 2012 4:45 AM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> On Tue, 2012-04-10 at 20:04 +0100, Ross Philipson wrote:
> > Not really. I think part of the problem here is my mixing of
> > terminology. For SMBIOS bits I am pulling apart the overall SMBIOS
> > table and just grabbing a desired subset of the SMBIOS structures. The
> > individual structures are the entities that do not have an overall
> > length field
> 
> So, to use the terminology of tools/firmware/hvmloader/smbios_types.h,
> you have entities which are subsets of a structure and so do not start
> with a "struct smbios_structure_header"?
> 
> Ian.
> 

Yea so the entire SMBIOS table begins with the "struct smbios_entry_point" which is not present in what I pass in. I have N structs (possibly some of the same type) that start with a "struct smbios_structure_header". The gotcha is that the "length" field only defines the fixed portion of each of the SMBIOS structs. There can be any number of strings following that struct of varying length. Then entire structure is doubly terminated with "\0\0". What I wanted to avoid is having to put the parsing code in hvmloader to reparse for each struct, which was already done by the tools. A simple approach is to use the <length><blob> scheme.

Thanks
Ross

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Wed, 2012-04-11 at 17:29 +0100, Ross Philipson wrote:
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: Wednesday, April 11, 2012 4:45 AM
> > To: Ross Philipson
> > Cc: xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > 
> > On Tue, 2012-04-10 at 20:04 +0100, Ross Philipson wrote:
> > > Not really. I think part of the problem here is my mixing of
> > > terminology. For SMBIOS bits I am pulling apart the overall SMBIOS
> > > table and just grabbing a desired subset of the SMBIOS structures. The
> > > individual structures are the entities that do not have an overall
> > > length field
> > 
> > So, to use the terminology of tools/firmware/hvmloader/smbios_types.h,
> > you have entities which are subsets of a structure and so do not start
> > with a "struct smbios_structure_header"?
> > 
> > Ian.
> > 
> 
> Yea so the entire SMBIOS table begins with the "struct
> smbios_entry_point" which is not present in what I pass in. I have N
> structs (possibly some of the same type) that start with a "struct
> smbios_structure_header". The gotcha is that the "length" field only
> defines the fixed portion of each of the SMBIOS structs. There can be
> any number of strings following that struct of varying length.

Ah, that's the bit I had forgotten...

>  Then entire structure is doubly terminated with "\0\0". What I wanted
> to avoid is having to put the parsing code in hvmloader to reparse for
> each struct, which was already done by the tools. A simple approach is
> to use the <length><blob> scheme.

Yes, I see now why this is necessary, thanks for plugging away at my
understanding ;-)

Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> >
> > Yea so the entire SMBIOS table begins with the "struct
> > smbios_entry_point" which is not present in what I pass in. I have N
> > structs (possibly some of the same type) that start with a "struct
> > smbios_structure_header". The gotcha is that the "length" field only
> > defines the fixed portion of each of the SMBIOS structs. There can be
> > any number of strings following that struct of varying length.
> 
> Ah, that's the bit I had forgotten...
> 
> >  Then entire structure is doubly terminated with "\0\0". What I wanted
> > to avoid is having to put the parsing code in hvmloader to reparse for
> > each struct, which was already done by the tools. A simple approach is
> > to use the <length><blob> scheme.
> 
> Yes, I see now why this is necessary, thanks for plugging away at my
> understanding ;-)
> 
> Ian.
> 

Sorry I did not get back sooner, I was out on vacation. So it sounds like I can make another go at this now. Briefly what I plan to do:
 - Pass in the firmware chunks as a list of blobs in struct xc_hvm_build_args to libxc
 - Return the base address where the chunks were loaded for each to the caller
 - The base address, size and checksum for each will be stored in xenstore
 - HVMLOADER will fetch the firmware chunks when building the tables etc.
 - I will address the issues brought up regarding the SMBIOS processing code.

Is it OK if I introduce a shared header in xen/include/public/hvm to collect all the BIOS/FW related xenstore values in one place? Something like hvm_fw_defs.h?

Thanks
Ross

Re: [PATCH 00/07] HVM firmware passthrough

[Tim Deegan]

Hi, 

At 14:47 -0400 on 25 Apr (1335365226), Ross Philipson wrote:
> Is it OK if I introduce a shared header in xen/include/public/hvm to
> collect all the BIOS/FW related xenstore values in one place?
> Something like hvm_fw_defs.h?

If at all possible, can you put that header somewhere under tools/ ?
AIUI it doesn't describe a hypervisor interface. 

Tim.

Re: [PATCH 00/07] HVM firmware passthrough

[Ross Philipson]

> -----Original Message-----
> From: Tim Deegan [mailto:tim@xen.org]
> Sent: Wednesday, April 25, 2012 4:48 PM
> To: Ross Philipson
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> 
> Hi,
> 
> At 14:47 -0400 on 25 Apr (1335365226), Ross Philipson wrote:
> > Is it OK if I introduce a shared header in xen/include/public/hvm to
> > collect all the BIOS/FW related xenstore values in one place?
> > Something like hvm_fw_defs.h?
> 
> If at all possible, can you put that header somewhere under tools/ ?
> AIUI it doesn't describe a hypervisor interface.
> 
> Tim.

Yea I can do that.

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Wed, 2012-04-25 at 22:05 +0100, Ross Philipson wrote:
> > -----Original Message-----
> > From: Tim Deegan [mailto:tim@xen.org]
> > Sent: Wednesday, April 25, 2012 4:48 PM
> > To: Ross Philipson
> > Cc: xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough
> > 
> > Hi,
> > 
> > At 14:47 -0400 on 25 Apr (1335365226), Ross Philipson wrote:
> > > Is it OK if I introduce a shared header in xen/include/public/hvm to
> > > collect all the BIOS/FW related xenstore values in one place?
> > > Something like hvm_fw_defs.h?
> > 
> > If at all possible, can you put that header somewhere under tools/ ?
> > AIUI it doesn't describe a hypervisor interface.
> > 
> > Tim.
> 
> Yea I can do that.

There isn't an existing good place which is shared between hvmloader and
libxc/libxl (other than public). Perhaps add a new subdir under
tools/include? xen-internal perhaps? xen-tools?

Ian.

Re: [PATCH 00/07] HVM firmware passthrough

[Julian Pidancet]

On 03/22/12 11:26, Tim Deegan wrote:
> Hi, 
> 
> At 09:24 +0000 on 20 Mar (1332235452), Tim Deegan wrote:
>> My impression from the earlier discussions is that we're pasing largish
>> blobs of binary BIOS goop around, which aren't suitable to go into
>> xenstore.  Dropping them in memory where HVMloader can pick them up
>> seems reasonable.
>>
>> All the control-path stuff - what the blobs are, where they are &c,
>> should go through Xenstore, though.
> 
> So having looked at the code, I think the module system is really
> overkill - AIUI all the things you're talking about passing through are
> ACPI tables, which have their own length fields internally.  So all
> you'd need to do is have a type code and an address in xenstore
> somewhere, the same way we pass a type code and a string for the other
> BIOS customizations.
> 
> Cheers,
> 
> Tim.

Hi,

I don't think ACPI and SMBIOS firmware passthrough are the only use
cases here.

The module architecture could also be used to pass the BIOS and Option
ROMs to hvmloader. The toolstack could load them from dom0's filesystem
dynamically and pass them to hvmloader, instead of having them
compiled-in statically in hvmloader.

Would we still be able to do that with the simplifications you're
suggesting here ?

-- 
Julian

Re: [PATCH 00/07] HVM firmware passthrough

[Tim Deegan]

At 20:47 +0100 on 01 May (1335905240), Julian Pidancet wrote:
> I don't think ACPI and SMBIOS firmware passthrough are the only use
> cases here.
> 
> The module architecture could also be used to pass the BIOS and Option
> ROMs to hvmloader. The toolstack could load them from dom0's filesystem
> dynamically and pass them to hvmloader, instead of having them
> compiled-in statically in hvmloader.

AFAIK the BIOS can already load option ROMs from real or emulated
hardware, the way a real BIOS does, and I think that's a good interface.

Is loading a custom BIOS (as opposed to option ROM) something you
actually want to do, BTW?

> Would we still be able to do that with the simplifications you're
> suggesting here ?

Yes, you could definitely do it without all this module stuff.  Passing
an address and length in Xenstore would work.

Tim.

Re: [PATCH 00/07] HVM firmware passthrough

[Julian Pidancet]

On Tue, May 1, 2012 at 11:17 PM, Tim Deegan <tim@xen.org> wrote:
> At 20:47 +0100 on 01 May (1335905240), Julian Pidancet wrote:
>> I don't think ACPI and SMBIOS firmware passthrough are the only use
>> cases here.
>>
>> The module architecture could also be used to pass the BIOS and Option
>> ROMs to hvmloader. The toolstack could load them from dom0's filesystem
>> dynamically and pass them to hvmloader, instead of having them
>> compiled-in statically in hvmloader.
>
> AFAIK the BIOS can already load option ROMs from real or emulated
> hardware, the way a real BIOS does, and I think that's a good interface.
>

That's true for SeaBIOS, but when using rombios, hvmloader loads the
VGA option ROM and etherboot into the VM's address space before
executing the BIOS.

> Is loading a custom BIOS (as opposed to option ROM) something you
> actually want to do, BTW?
>

Yes.

>> Would we still be able to do that with the simplifications you're
>> suggesting here ?
>
> Yes, you could definitely do it without all this module stuff.  Passing
> an address and length in Xenstore would work.
>

Isn't it kind of the same thing ? I mean, the BIOS or an Option ROM to
pre-load could be seen by hvmloader as "modules" because they would
have to be passed by the toolstack the same way as ACPI or SMBIOS
tables.

-- 
Julian

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 00/07] HVM firmware passthrough

[Tim Deegan]

At 23:59 +0100 on 01 May (1335916778), Julian Pidancet wrote:
> >> Would we still be able to do that with the simplifications you're
> >> suggesting here ?
> >
> > Yes, you could definitely do it without all this module stuff.  Passing
> > an address and length in Xenstore would work.
> 
> Isn't it kind of the same thing ? I mean, the BIOS or an Option ROM to
> pre-load could be seen by hvmloader as "modules" because they would
> have to be passed by the toolstack the same way as ACPI or SMBIOS
> tables.

Yes, it's similar.  But the new module marshalling/unmarshalling code
isn't necessary for any of them.

Tim.

Re: [PATCH 00/07] HVM firmware passthrough

[Ian Campbell]

On Wed, 2012-05-02 at 00:21 +0100, Tim Deegan wrote:
> At 23:59 +0100 on 01 May (1335916778), Julian Pidancet wrote:
> > >> Would we still be able to do that with the simplifications you're
> > >> suggesting here ?
> > >
> > > Yes, you could definitely do it without all this module stuff.  Passing
> > > an address and length in Xenstore would work.
> > 
> > Isn't it kind of the same thing ? I mean, the BIOS or an Option ROM to
> > pre-load could be seen by hvmloader as "modules" because they would
> > have to be passed by the toolstack the same way as ACPI or SMBIOS
> > tables.
> 
> Yes, it's similar.  But the new module marshalling/unmarshalling code
> isn't necessary for any of them.

Agreed, if necessary then there should be specific fields in
xc_hvm_build_args to provide these overrides.

I'm not totally convinced about adding new features (specifically Option
ROM deployment support) for ROMBIOS+qemu-xen-traditional. By the time
this stuff hits the tree (i.e. 4.3 at this point) we should have
switched to SEABIOS+qemu-xen-upstream as our default BIOS+ioemu pair. It
is already the case that we have been refusing new features to
qemu-xen-traditional but we haven't yet extended this to ROMBIOS (I
don't think). I suppose the flip side is that the code ought to be small
and relatively self contained under if(rombios) conditionals in
hvmloader, so long as we make it clear in the xc interface that those
ROMs only apply if you are using ROMBIOS.

Ian.