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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



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