Re: [Xen-devel] [PATCH 00/07] HVM firmware passthrough

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxx]
> > Sent: Thursday, March 22, 2012 7:26 AM
> > To: Keir Fraser
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; 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.



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