Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

On 06/14/13 02:26, Laszlo Ersek wrote:
> On 06/14/13 01:02, Paolo Bonzini wrote:
>> Il 10/06/2013 21:03, Anthony Liguori ha scritto:
> I'm not really convinced that
> QEMU<->firmware is a GPL boundary because of how tightly the two are
> linked.

 Where has 'linked' in terms of the GPL ever been anything other than
 actual executable linking?
>>>
>>> I should not have even brought this up as it's not worth debating.  If
>>> you're curious, http://www.gnu.org/licenses/gpl-faq.html#MereAggregation
>>
>> With the usual IANAL care, I think QOM would be much much more of a
>> legal grey area that passing ACPI tables.
>>
>> If you pass ACPI tables, the ACPI tables are clearly part of QEMU, and
>> are almost as clearly "just data" for the BIOS.
>>
>> But if you use QOM, you may start wondering if "the semantics of the
>> communication are intimate enough, exchanging complex internal data
>> structures".  Probably not, as it is a generic interface and there could
>> be in principle other consumers than the firmware, but still complex
>> enough that raising the issue is not moot.
> 
> Basing the decision about
> 
> derivative or not
> 
> on
> 
> internal
> 
> or
> 
> complex enough
> 
> ; well I find that unusable.
> 
> First, complexity: web services can use very complex XML schemas, and
> that clearly doesn't make the server derivative of the client, or vice
> versa.
> 
> Second, internality: this attribute can be wiped out simply by writing
> documentation for the data structure (which should be done *anyway*).
> Once it is documented, it is not internal any longer. However existence
> of documentation for a wire format between A and B should have
> absolutely no say in whether codebase A is derivative of codebase B.
> 
> IANAL of course but I find the FSF's argument biased.
> 
> (Of course I'm also not buying that linking against a library makes the
> client application (its own source code, or its dynamically linked
> binary) derivative of the library. If there are two libraries
> (especially when released under different licenses) implementing the
> same API, which one is the application a derivative of?)

This is my personal, private opinion, of course, which is independent of
what my employer holds.

Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Paolo Bonzini  writes:

> Il 11/06/2013 03:35, Michael S. Tsirkin ha scritto:
>> Two points
>> 1. You never explained what you mean by un-hardware like.
>> 
>>Currently bios is in a ROM device, and it has a
>>template for ACPI tables together with it.
>>This simply moves the tables to a separate ROM
>>device (FW CFG), and generalizes the template using
>>the linker interface.
>>One ROM is hardware-like but two is un-hardware like?
>> 
>>ACPI tables are static so it's likely lots of
>>hardware has at least some of them pre-formatted in flash,
>>then tweak some things like SRAT a bit.
>
> Also having a "bootstrap processor" was certainly not unheard of some
> decades ago.  Right now we get all sort of SMM hacks instead of adding
> more processors, but it's certainly not un-hardware like.

It's still not unheard of.  This is how power systems work still.

However, with PCs, the ACPI tables are generated by/included in the
firmware.  There's no question about that.

>
> Maybe we should just have a bytecode interpreter and write the ACPI
> generator in that language. :)

Indeed, we can even using an existing bytecode like the x86 instruction
set and use this VM called KVM to execute it.  I hear there are even C
compilers for this bytecode ;-)

Regards,

Anthony Liguori

> Paolo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

On 06/14/13 01:02, Paolo Bonzini wrote:
> Il 10/06/2013 21:03, Anthony Liguori ha scritto:
 I'm not really convinced that
 QEMU<->firmware is a GPL boundary because of how tightly the two are
 linked.
>>>
>>> Where has 'linked' in terms of the GPL ever been anything other than
>>> actual executable linking?
>>
>> I should not have even brought this up as it's not worth debating.  If
>> you're curious, http://www.gnu.org/licenses/gpl-faq.html#MereAggregation
> 
> With the usual IANAL care, I think QOM would be much much more of a
> legal grey area that passing ACPI tables.
> 
> If you pass ACPI tables, the ACPI tables are clearly part of QEMU, and
> are almost as clearly "just data" for the BIOS.
> 
> But if you use QOM, you may start wondering if "the semantics of the
> communication are intimate enough, exchanging complex internal data
> structures".  Probably not, as it is a generic interface and there could
> be in principle other consumers than the firmware, but still complex
> enough that raising the issue is not moot.

Basing the decision about

derivative or not

on

internal

or

complex enough

; well I find that unusable.

First, complexity: web services can use very complex XML schemas, and
that clearly doesn't make the server derivative of the client, or vice
versa.

Second, internality: this attribute can be wiped out simply by writing
documentation for the data structure (which should be done *anyway*).
Once it is documented, it is not internal any longer. However existence
of documentation for a wire format between A and B should have
absolutely no say in whether codebase A is derivative of codebase B.

IANAL of course but I find the FSF's argument biased.

(Of course I'm also not buying that linking against a library makes the
client application (its own source code, or its dynamically linked
binary) derivative of the library. If there are two libraries
(especially when released under different licenses) implementing the
same API, which one is the application a derivative of?)

Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Paolo Bonzini]

Il 11/06/2013 03:35, Michael S. Tsirkin ha scritto:
> Two points
> 1. You never explained what you mean by un-hardware like.
> 
>Currently bios is in a ROM device, and it has a
>template for ACPI tables together with it.
>This simply moves the tables to a separate ROM
>device (FW CFG), and generalizes the template using
>the linker interface.
>One ROM is hardware-like but two is un-hardware like?
> 
>ACPI tables are static so it's likely lots of
>hardware has at least some of them pre-formatted in flash,
>then tweak some things like SRAT a bit.

Also having a "bootstrap processor" was certainly not unheard of some
decades ago.  Right now we get all sort of SMM hacks instead of adding
more processors, but it's certainly not un-hardware like.

Maybe we should just have a bytecode interpreter and write the ACPI
generator in that language. :)

Paolo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Paolo Bonzini]

Il 10/06/2013 21:03, Anthony Liguori ha scritto:
>>> I'm not really convinced that
>>> QEMU<->firmware is a GPL boundary because of how tightly the two are
>>> linked.
>>
>> Where has 'linked' in terms of the GPL ever been anything other than
>> actual executable linking?
> 
> I should not have even brought this up as it's not worth debating.  If
> you're curious, http://www.gnu.org/licenses/gpl-faq.html#MereAggregation

With the usual IANAL care, I think QOM would be much much more of a
legal grey area that passing ACPI tables.

If you pass ACPI tables, the ACPI tables are clearly part of QEMU, and
are almost as clearly "just data" for the BIOS.

But if you use QOM, you may start wondering if "the semantics of the
communication are intimate enough, exchanging complex internal data
structures".  Probably not, as it is a generic interface and there could
be in principle other consumers than the firmware, but still complex
enough that raising the issue is not moot.

Paolo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[David Woodhouse]

On Mon, 2013-06-10 at 19:11 -0500, Anthony Liguori wrote:
> If we did this right in QEMU, we'd have to introduce a QOM property
> anyway 

... and then we'd have to update each firmware implementation to know
about this new property, and the how it translates to the RESET_REG
field in the DSDT, etc.

So we *still* end up having to update firmwares to keep up with qemu,
much more than we would if we'd generated the tables on the qemu side.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Tue, Jun 11, 2013 at 11:18:13AM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 11, 2013 at 10:00:36AM +0200, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > Yes and not just because of windows guests.
> > > ACPI spec is also very explicit that native hotplug is an optional
> > > feature. Test suites such as WHQL are known to test spec compliance.
> > 
> > /me looks a bit surprised.
> > 
> > This pretty much implies that any shpc bridge needs a second interface
> > to the hotplug functionality which can be driven via ACPI.  Or the
> > firmware somehow handles this using smm (not sure this works for the IRQ
> > though).
> > 
> > Do you know how this is handled by real hardware?
> > 
> > cheers,
> >   Gerd
> 
> SHPC is not very widely deployed on a PC.
> 
> Since most hardware vendors do care about windows support,
> the only way is a separate interface that is driven via ACPI.
> You then need an ACPI specific register to switch to standard SHPC.
> The SHPC spec even tells you as much.

Just to give another example, interface in this patch
scales trivially to multi-root configurations,
if we ever want to support them.

> -- 
> MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Tue, Jun 11, 2013 at 10:00:36AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Yes and not just because of windows guests.
> > ACPI spec is also very explicit that native hotplug is an optional
> > feature. Test suites such as WHQL are known to test spec compliance.
> 
> /me looks a bit surprised.
> 
> This pretty much implies that any shpc bridge needs a second interface
> to the hotplug functionality which can be driven via ACPI.  Or the
> firmware somehow handles this using smm (not sure this works for the IRQ
> though).
> 
> Do you know how this is handled by real hardware?
> 
> cheers,
>   Gerd

SHPC is not very widely deployed on a PC.

Since most hardware vendors do care about windows support,
the only way is a separate interface that is driven via ACPI.
You then need an ACPI specific register to switch to standard SHPC.
The SHPC spec even tells you as much.

-- 
MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Gerd Hoffmann]

  Hi,

> Yes and not just because of windows guests.
> ACPI spec is also very explicit that native hotplug is an optional
> feature. Test suites such as WHQL are known to test spec compliance.

/me looks a bit surprised.

This pretty much implies that any shpc bridge needs a second interface
to the hotplug functionality which can be driven via ACPI.  Or the
firmware somehow handles this using smm (not sure this works for the IRQ
though).

Do you know how this is handled by real hardware?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Tue, Jun 11, 2013 at 09:42:29AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >>> Portability:
> >>> - Non x86 (or any Linux) platforms don't need any of this code.
> >>>   They can keep happily using SHPC the way
> >>>   they always did.
> >>
> >> Hmm.  Is is possible to write a SHPC driver in AML?  I think it would be
> >> alot better to have one guest/host interface for pci bridge hotplug
> >> instead of two.
> >>
> >> cheers,
> >>   Gerd
> > 
> > No, it's not possible, SHPC is not designed to be used from ACPI.
> > 
> > Two reasons off the top of my head, there are likely others:
> > 
> > 1. SHPC uses regular PCI interrupts to signal events. It does not signal
> >GFE and SCI.
> > 
> > 2. SHPC uses config accesses to get information from device.
> >ACPI does not allow config access anywhere except the root bus from ACPI
> >(This requirement is designed to give the OS freedom
> >to reconfigure PCI in an arbitrary way).
> 
> OK, so it's designed for OSes to have native SHPC support.  Linux has that?

Yes.

> Quick googling found me Windows Vista+ has it too, correct?  So that
> leaves Win2k + WinXP versions.  Older Windows versions do not support
> pci hotplug at all.  Win2k is EOL already.  WinXP will follow soon.
> 
> More users?

googling lead you astray.
No windows version supports SHPC.

> /me wonders whenever it is worth hopping through the loops needed to
> support ACPI-based hotplug of devices behind bridges in the first place.
> 
> cheers,
>   Gerd

Yes and not just because of windows guests.
ACPI spec is also very explicit that native hotplug is an optional
feature. Test suites such as WHQL are known to test spec compliance.

-- 
MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Gerd Hoffmann]

  Hi,

>>> Portability:
>>> - Non x86 (or any Linux) platforms don't need any of this code.
>>>   They can keep happily using SHPC the way
>>>   they always did.
>>
>> Hmm.  Is is possible to write a SHPC driver in AML?  I think it would be
>> alot better to have one guest/host interface for pci bridge hotplug
>> instead of two.
>>
>> cheers,
>>   Gerd
> 
> No, it's not possible, SHPC is not designed to be used from ACPI.
> 
> Two reasons off the top of my head, there are likely others:
> 
> 1. SHPC uses regular PCI interrupts to signal events. It does not signal
>GFE and SCI.
> 
> 2. SHPC uses config accesses to get information from device.
>ACPI does not allow config access anywhere except the root bus from ACPI
>(This requirement is designed to give the OS freedom
>to reconfigure PCI in an arbitrary way).

OK, so it's designed for OSes to have native SHPC support.  Linux has that?

Quick googling found me Windows Vista+ has it too, correct?  So that
leaves Win2k + WinXP versions.  Older Windows versions do not support
pci hotplug at all.  Win2k is EOL already.  WinXP will follow soon.

More users?

/me wonders whenever it is worth hopping through the loops needed to
support ACPI-based hotplug of devices behind bridges in the first place.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Mon, Jun 10, 2013 at 08:03:46PM -0500, Anthony Liguori wrote:
> >> It is not "supported" by QEMU.
> >
> > No, but I've always thought that QEMU was happy to have alternative
> > firmware projects.
> 
> We are and we're happy to accept patches to enable things even if its
> proprietary.  But that's all assuming we're improving hardware
> emulation.
> 
> What we're talking about here is doing something that's very un-hardware like.

Two points
1. You never explained what you mean by un-hardware like.

   Currently bios is in a ROM device, and it has a
   template for ACPI tables together with it.
   This simply moves the tables to a separate ROM
   device (FW CFG), and generalizes the template using
   the linker interface.
   One ROM is hardware-like but two is un-hardware like?

   ACPI tables are static so it's likely lots of
   hardware has at least some of them pre-formatted in flash,
   then tweak some things like SRAT a bit.

2. There's no hardware-like alternative.

   A current alternative is exposing lots of PV interfaces to bios,
   bios builds up ACPI tables based on them.
   All of this code is in practice PC only,
   if we ever attempt to generalize we'll see how
   the interfaces are not a good match.
   They are still PV, poorly documented and it's very hard for
   bios not to make any assumptions about them.
   When it does, result is a very obscure breakage.

   Yes, building tables in bios means
   QEMU code is simpler but we double the
   amount of interfaces that we need to get right:
   QEMU-BIOS + BIOS-OS

   Yes it's "not QEMU code" but it's seen as
   a single package by users and it does not really
   matter - excellent hardware with lousy firmware is
   useless.

-- 
MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Tue, Jun 11, 2013 at 07:33:02AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >* Use of glib's GArray makes it much easier to build
> >  up tables in code without need for iasl and code patching
> 
> Nice.
> 
> > Design:
> > - each bus gets assigned a number 0-255
> > - generated ACPI code writes this number
> >   to a new BSEL register, then uses existing
> >   UP/DOWN registers to probe slot status;
> >   to eject, write number to BSEL register,
> >   then slot into existing EJ
> > 
> > This is to address the ACPI spec requirement to
> > avoid config cycle access to any bus except PCI roots.
> > 
> > Portability:
> > - Non x86 (or any Linux) platforms don't need any of this code.
> >   They can keep happily using SHPC the way
> >   they always did.
> 
> Hmm.  Is is possible to write a SHPC driver in AML?  I think it would be
> alot better to have one guest/host interface for pci bridge hotplug
> instead of two.
> 
> cheers,
>   Gerd

No, it's not possible, SHPC is not designed to be used from ACPI.

Two reasons off the top of my head, there are likely others:

1. SHPC uses regular PCI interrupts to signal events. It does not signal
   GFE and SCI.

2. SHPC uses config accesses to get information from device.
   ACPI does not allow config access anywhere except the root bus from ACPI
   (This requirement is designed to give the OS freedom
   to reconfigure PCI in an arbitrary way).

-- 
MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Mon, Jun 10, 2013 at 08:25:15PM -0500, Anthony Liguori wrote:
> > I do understand your desire to pass this stuff as parameters, but I
> > really don't see it as feasible.  I'm hoping that if you can write up
> > some examples with specifics you'll either enlighten me or you'll see
> > the difficulties I'm trying to describe.
> 
> Is your basic argument that generating these tables is hard, QEMU can
> and should just hard code them and change them whenever it feels like
> without involving SeaBIOS?

Basically I think it makes sense to listen to people
that write drivers for PV hardware that QEMU exposes.
bios is one such example.

If people that are supposed to use our interfaces
ask for a specific interface, I think we should listen.
If people come and say that building acpi dynamically is hard
for them and they want QEMU to prepare templates for acpi -
and there's overwhelming concensus on this -
QEMU should stop going "tough luck we know what's best for you".

> If that's what it boils down to, I'll look more closely at it.

So please look at the original patchset
[PATCH RFC 00/13] qemu: generate acpi tables for the guest

and let's move the discussion of it beyond the meta-argument
"QEMU does not need to generate any ACPI tables" 
and to the specific interface that we can use
to make life easier for the bios.

Specifically several options were discussed, on and off-line for passing
per-formatted tables to firmware:

1. use existing fw cfg interface from bios,
   qemu side make it work well with cross version migration.
2. same as 1 but find some existing hardware that is similar
   to fw cfg and emulate it. For example build a directory
   structure in some flash.
3. make qemu put tables into guest memory.
   bios needs to reserve this memory.
4. same as 3 but bios allocates memory and
   passes address to qemu.

Kevin suggested 1. This is what my patchset implements.
But I think bios could live with any of
the others, as long as it's a generic interface so
we can stop changing bios code all the time.

-- 
MST

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Gerd Hoffmann]

  Hi,

>* Use of glib's GArray makes it much easier to build
>  up tables in code without need for iasl and code patching

Nice.

> Design:
> - each bus gets assigned a number 0-255
> - generated ACPI code writes this number
>   to a new BSEL register, then uses existing
>   UP/DOWN registers to probe slot status;
>   to eject, write number to BSEL register,
>   then slot into existing EJ
> 
> This is to address the ACPI spec requirement to
> avoid config cycle access to any bus except PCI roots.
> 
> Portability:
> - Non x86 (or any Linux) platforms don't need any of this code.
>   They can keep happily using SHPC the way
>   they always did.

Hmm.  Is is possible to write a SHPC driver in AML?  I think it would be
alot better to have one guest/host interface for pci bridge hotplug
instead of two.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Michael S. Tsirkin]

On Mon, Jun 10, 2013 at 01:58:41PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > This adds support for device hotplug behind
> > pci bridges. Bridge devices themselves need
> > to be pre-configured on qemu command line.
> >
> > One of the goals of this project was to
> > demonstrate the advantages of generating
> > ACPI tables in QEMU.
> > In my opinion, it does this successfully.
> 
> Since you've gone out of your way to make this difficult to actually
> review...

What's wrong?

> >
> >* This saves the need to provide a new interface for firmware
> >  to discover bus number to pci brige mapping
> 
> Do you mean fw_cfg?  The BIOS normally does bus numbering.  I see no
> reason for it not to.

I did my best to explain this. ACPI spec explicitly states
config access to devices not on bus 0 is forbidden.
This is to allow OS to renumber the bus at any time.
So you have pci bus number but can't use it.

> >* No need for yet another interface to bios to detect qemu version
> >  to check if it's safe to activate new code,
> >  and to ship multiple table versions:
> 
> We only care about supporting one version of SeaBIOS.  SeaBIOS should
> only care about supporting one version of QEMU.  We're not asking it to
> support multiple versions.

We do e.g. when we run with -M1.2 and migrate to old QEMU
early in boot process.


> >  we generated code so we know this is new qemu
> >* Use of glib's GArray makes it much easier to build
> >  up tables in code without need for iasl and code patching
> 
> Adding a dynamic array to SeaBIOS isn't rocket science.

Glib reimplementation in bios will be more code than using existing
glib.

> >
> > I have also tried to address the concerns that
> > Anthony has raised with this approach, please see below.
> >
> > Design:
> > - each bus gets assigned a number 0-255
> > - generated ACPI code writes this number
> >   to a new BSEL register, then uses existing
> >   UP/DOWN registers to probe slot status;
> >   to eject, write number to BSEL register,
> >   then slot into existing EJ
> >
> > This is to address the ACPI spec requirement to
> > avoid config cycle access to any bus except PCI roots.
> >
> > Portability:
> > - Non x86 (or any Linux) platforms don't need any of this code.
> >   They can keep happily using SHPC the way
> >   they always did.
> >
> > Things to note:
> > - this is on top of acpi patchset posted a month ago,
> >   with a small patch adding a core function to walk all
> >   pci buses, on top.
> >   Can also be found in my git tree
> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi
> >
> > - Extensive use of glib completely removes
> >   pointer math in new code: we use
> >   g_array_append_vals exclusively.
> >   There's no code patching in new code.
> >   This is in response to comments about security
> >   concerns with adding code to qemu.
> >   In this sense it is more secure than existing
> >   code in hw/acpi/core.c - that can be switched to
> >   glib if desired.
> >
> > - New code (not imported from seabios)
> >   uses glib to reduce dependency on iasl.
> >   With time all code can be rewritted to
> >   use glib and avoid iasl, if desired.
> >
> > - As was the case previously,
> >   systems that lack working iasl are detected at configure time,
> >   pre-generated hex files in source tree are used in this case.
> >   This addresses the concern with iasl/big-endian
> >   systems.
> >
> > - Compile tested only. Migration is known to be
> >   broken - there are a bunch of TODO tags in code.
> >   It was agreed on the last qemu conf meeting that
> >   this code is posted without looking at migration,
> >   with understanding that it is addressed before
> >   being merged. Please do not mistake this
> >   limitation for a fundamental one - I have a
> >   very good idea how to fix it, including
> >   cross-version migration.
> >
> > - Cross version migration: when running with -M 1.5
> >   and older, all ACPI table generation should be disabled.
> >   We'll present FW_CFG interface compatible with 1.5.
> 
> So TL;DR, you break a bunch of stuff

Confused.
When you asked me to write this up during last conf call,
you explicitly said not to bother with migration?
Almost any change affecting guest needs migration
related work.

> and introduce a mess of code.

What did you expect? New functionality with less code?

>  It
> would be less code and wouldn't break anything to add this logic to
> SeaBIOS.

It will be much more code - we would need to add a bunch of interfaces
to qemu then code using that to seabios, worry about running this
seabios on old qemu, etc etc.

> How is this even a discussion?

Interface change is minimized.

Interface to bios is not changed at all.
Interface to gu

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Kevin O'Connor]

On Mon, Jun 10, 2013 at 08:25:15PM -0500, Anthony Liguori wrote:
> On Mon, Jun 10, 2013 at 8:19 PM, Kevin O'Connor  wrote:
> > I do understand your desire to pass this stuff as parameters, but I
> > really don't see it as feasible.  I'm hoping that if you can write up
> > some examples with specifics you'll either enlighten me or you'll see
> > the difficulties I'm trying to describe.
> 
> Is your basic argument that generating these tables is hard, QEMU can
> and should just hard code them and change them whenever it feels like
> without involving SeaBIOS?

Yes - that's my basic argument.  Generating the content in a way that
is future proof (or future flexible) is very hard.  The DSDT is
particularly painful.  I'm also afraid of introducing a lot of
complexity when simply hardcoding the content (in the right place) is
the better option.

> If that's what it boils down to, I'll look more closely at it.

Thanks,
-Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Jordan Justen]

On Mon, Jun 10, 2013 at 6:03 PM, Anthony Liguori  wrote:
> On Mon, Jun 10, 2013 at 7:28 PM, Jordan Justen  wrote:
>> It couldn't hurt if more people that actually care about this spoke up
>> on edk2-devel about the issue, or perhaps within a UEFI working group.
>> Because, I know that they've stopped listening to me about it.
>
> Is this useful?  I can try to make noise.

Possibly. But, I don't want to give you false hope. :)

> I assume since folks like
> you who have much more credibility and familiarity in this space have
> given up, it's a lost cause.

I think customers/partners reporting an issue directly can sometime be
more persuasive than a lone engineer saying this somewhat obscure
licensing thing will be an issue for free software projects.

>> No, but I've always thought that QEMU was happy to have alternative
>> firmware projects.
>
> We are and we're happy to accept patches to enable things even if its
> proprietary.  But that's all assuming we're improving hardware
> emulation.
>
> What we're talking about here is doing something that's very un-hardware like.

Fair enough. I know that it's been done by other VMs, but you
certainly have a point in that real systems don't do this.

-Jordan

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Hi Kevin,

On Mon, Jun 10, 2013 at 8:19 PM, Kevin O'Connor  wrote:
> On Mon, Jun 10, 2013 at 07:51:55PM -0500, Anthony Liguori wrote:
>> I think that we can pretty much touch a table once pulling all of the
>> info from QOM and then from a SeaBIOS point of view, never have to
>> touch it again.
>
> Thanks.  I do think it would help if you could go through the details
> of a couple of tables and a couple of DSDT entries and walk through
> where each field will come from.  And also describe how seabios will
> build the resulting AML.  From my previous email:
>
>> I'm quite curious how you are planning to solve (a).  I think it would
>> help move this conversation forward if you could take a couple acpi
>> tables in use today (eg, madt, srat) and describe the future format
>> and location for each field in those tables.  I think it would also be
>> useful if you could do the same for a couple DSDT entries (eg,
>> \_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
>> guest build the AML from that info.
>
> I do understand your desire to pass this stuff as parameters, but I
> really don't see it as feasible.  I'm hoping that if you can write up
> some examples with specifics you'll either enlighten me or you'll see
> the difficulties I'm trying to describe.

Is your basic argument that generating these tables is hard, QEMU can
and should just hard code them and change them whenever it feels like
without involving SeaBIOS?

If that's what it boils down to, I'll look more closely at it.

Regards,

Anthony Liguori

> -Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Kevin O'Connor]

On Mon, Jun 10, 2013 at 07:51:55PM -0500, Anthony Liguori wrote:
> I think that we can pretty much touch a table once pulling all of the
> info from QOM and then from a SeaBIOS point of view, never have to
> touch it again.

Thanks.  I do think it would help if you could go through the details
of a couple of tables and a couple of DSDT entries and walk through
where each field will come from.  And also describe how seabios will
build the resulting AML.  From my previous email:

> I'm quite curious how you are planning to solve (a).  I think it would
> help move this conversation forward if you could take a couple acpi
> tables in use today (eg, madt, srat) and describe the future format
> and location for each field in those tables.  I think it would also be
> useful if you could do the same for a couple DSDT entries (eg,
> \_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
> guest build the AML from that info.

I do understand your desire to pass this stuff as parameters, but I
really don't see it as feasible.  I'm hoping that if you can write up
some examples with specifics you'll either enlighten me or you'll see
the difficulties I'm trying to describe.

-Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Hi Jordan,

On Mon, Jun 10, 2013 at 7:28 PM, Jordan Justen  wrote:
> On Mon, Jun 10, 2013 at 2:45 PM, Anthony Liguori  
> wrote:
>> OVMF is proprietary.
>
> I don't agree that not-OSI means proprietary.

I call it blue if that makes you all feel better :-)  But no matter
what the word we use it, the point still stands, we need a UEFI
firmware that has a OSI-approved license.  Forgetting everything else,
the distros can't ship OVMF in its current form.

> I agree that the FAT driver is not 'free software' and I agree that is
> a problem for usage with free software projects, such as QEMU. This is
> a big deal, but unfortunately, as an Intel employee, I think I've done
> as much as I can to address this.
>
> It couldn't hurt if more people that actually care about this spoke up
> on edk2-devel about the issue, or perhaps within a UEFI working group.
> Because, I know that they've stopped listening to me about it.

Is this useful?  I can try to make noise.  I assume since folks like
you who have much more credibility and familiarity in this space have
given up, it's a lost cause.

>> It is not "supported" by QEMU.
>
> No, but I've always thought that QEMU was happy to have alternative
> firmware projects.

We are and we're happy to accept patches to enable things even if its
proprietary.  But that's all assuming we're improving hardware
emulation.

What we're talking about here is doing something that's very un-hardware like.

>> I'm not really convinced that
>> QEMU<->firmware is a GPL boundary because of how tightly the two are
>> linked.
>
> Where has 'linked' in terms of the GPL ever been anything other than
> actual executable linking?

I should not have even brought this up as it's not worth debating.  If
you're curious,
http://www.gnu.org/licenses/gpl-faq.html#MereAggregation

>> Moving large chunks of firmware code into QEMU just to avoid solving
>> licensing issues is a non-starter with me.
>
> Is this a licensing issue? I thought this was a "let's save time by
> doing it in one place" thing. I'm pretty ambivalent about this
> feature, really. I don't think it is even worth all this bickering.
>
> I'm certain OVMF has ACPI issues on QEMU, but I don't think it is a
> huge deal to resolve them independently of this feature.

Doing it one place could just mean share code.  Code can't be shared
today, that's why licensing has come up.

Regards,

Anthony Liguori

> I was not a huge fan of supporting this type of thing for Xen in OVMF,
> but it does seem to work fine.
>
> -Jordan

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Hi Kevin,

On Mon, Jun 10, 2013 at 7:23 PM, Kevin O'Connor  wrote:
> On Mon, Jun 10, 2013 at 06:34:29PM -0500, Anthony Liguori wrote:
>> Kevin O'Connor  writes:
>>
>> For the MADT table, right now SeaBIOS needs the CPU count.  That can be
>> found by counting the number of CPU nodes.  Today cpus are unattached so
>> they appear in /machine/unattached but we should put them in a
>> /machine/cpu container for clarity.
>
> SeaBIOS needs much more than the CPU count.  The madt contains info on
> each interrupt - its global irq number, the legacy irq number, the
> acpi defined type of the irq, and the acpi defined flags for the irq.
> It also contains similar info on each cpu (including apic id), the io
> apic, and NMIs.

See below.

>> QOM is the full representation of the device state so we should not ever
>> need to add additional things to fw_cfg.  More likely than not, when
>> SeaBIOS needs more information, it's already there so added
>> functionality will probably Just Work with older QEMUs.
>>
>> > I think it would also be
>> > useful if you could do the same for a couple DSDT entries (eg,
>> > \_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
>> > guest build the AML from that info.
>>
>> Likewise the slot count should be available too.  We hard code slots
>> today but it is something we should model properly.  We would model it
>> using QOM of course.
>>
>> Internally within QEMU, this initial discussion started by saying that
>> any ACPI generation within QEMU should happen strictly with QOM
>> methods.  This was the crux of my argument, if QOM is the only interface
>> we need, everything is there for the firmware to do the same job that
>> QEMU would be doing.
>
> I think we keep talking past each other.  You seem to be pointing out
> that the information seabios uses to patch its hardcoded tables can be
> passed in via QOM.  Agreed, it can.  I'm pointing out all the info
> that is hardcoded in seabios - I don't see how that can be passed via
> QOM.

No, we aren't talking past each other.  We both have the same goal.

A lot of what is hard coded in SeaBIOS is hard coded in QEMU too.  IRQ
routing is a good example of that.  We want to make this information
dynamic.

Part of the trouble is that we haven't had a need to not hard code it
because this is only information consumed by the BIOS.

> All the hardcoded data in seabios is a problem, because when it
> changes (and it frequently does) it requires changes to both QEMU and
> SeaBIOS and those changes have to be coordinated.  The key reason for
> moving the tables into QEMU is not that it can better patch the tables
> - the advantage is that it can hardcode the tables that need patching.

So let's fix it.  It's very easy to add read-only properties to QOM so
we can hard code the bits that are in SeaBIOS now as read-only
properties.  For the MADT table, the per-CPU and IOAPIC info can
simply be properties of those devices.  We already represent irqs in
QOM as integers so I suspect much of the information SeaBIOS needs is
already there.

> I can cite several recent examples of seabios change requests that
> require changing of the hardcoded tables: liguang wants to add an
> "embedded controller" hardware device which requires changes to
> seabios' dsdt, Corey Minyard wants to add IPMI hardware support (both
> smbios changes and DSDT changes), and Corey Bryant wants to add TPM
> hardware support.
>
> How do we solve the problem of seabios having a ton of hardcoded
> information about the qemu hardware, and seabios having to change with
> the hardware modifications that qemu makes?

I think that we can pretty much touch a table once pulling all of the
info from QOM and then from a SeaBIOS point of view, never have to
touch it again.

The benefit is that solving this problem for x86 solves it for other
architectures too and also lays the ground work to let a user actually
control these bits too.

Regards,

Anthony Liguori

> -Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Jordan Justen]

On Mon, Jun 10, 2013 at 2:45 PM, Anthony Liguori  wrote:
> OVMF is proprietary.

I don't agree that not-OSI means proprietary.

I agree that the FAT driver is not 'free software' and I agree that is
a problem for usage with free software projects, such as QEMU. This is
a big deal, but unfortunately, as an Intel employee, I think I've done
as much as I can to address this.

It couldn't hurt if more people that actually care about this spoke up
on edk2-devel about the issue, or perhaps within a UEFI working group.
Because, I know that they've stopped listening to me about it.

> It is not "supported" by QEMU.

No, but I've always thought that QEMU was happy to have alternative
firmware projects.

> I'm not really convinced that
> QEMU<->firmware is a GPL boundary because of how tightly the two are
> linked.

Where has 'linked' in terms of the GPL ever been anything other than
actual executable linking?

> Moving large chunks of firmware code into QEMU just to avoid solving
> licensing issues is a non-starter with me.

Is this a licensing issue? I thought this was a "let's save time by
doing it in one place" thing. I'm pretty ambivalent about this
feature, really. I don't think it is even worth all this bickering.

I'm certain OVMF has ACPI issues on QEMU, but I don't think it is a
huge deal to resolve them independently of this feature.

I was not a huge fan of supporting this type of thing for Xen in OVMF,
but it does seem to work fine.

-Jordan

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Kevin O'Connor]

On Mon, Jun 10, 2013 at 06:34:29PM -0500, Anthony Liguori wrote:
> Kevin O'Connor  writes:
> 
> > On Mon, Jun 10, 2013 at 04:45:53PM -0500, Anthony Liguori wrote:
> >> This discussion comes down to two things I think: (a) our existing
> >> firmware interface is pretty poor (b) we are duplicating work because of
> >> firmware licensing.
> >> 
> >> We can fix (a) and there's lots of value in doing that in terms of
> >> improving support for other architectures.  We've discussed (b) in other
> >> threads and I've stated my opinion on the direction we need to take.
> >
> > I'm not concerned about (b).
> >
> > I'm quite curious how you are planning to solve (a).  I think it would
> > help move this conversation forward if you could take a couple acpi
> > tables in use today (eg, madt, srat) and describe the future format
> > and location for each field in those tables.
> 
> Sure.  Today we expose the device model through a graph that can be
> losely mapped to a filesystem.  We expose two methods of interest as
> part of our management api:
[...]
> For the MADT table, right now SeaBIOS needs the CPU count.  That can be
> found by counting the number of CPU nodes.  Today cpus are unattached so
> they appear in /machine/unattached but we should put them in a
> /machine/cpu container for clarity.

SeaBIOS needs much more than the CPU count.  The madt contains info on
each interrupt - its global irq number, the legacy irq number, the
acpi defined type of the irq, and the acpi defined flags for the irq.
It also contains similar info on each cpu (including apic id), the io
apic, and NMIs.

> QOM is the full representation of the device state so we should not ever
> need to add additional things to fw_cfg.  More likely than not, when
> SeaBIOS needs more information, it's already there so added
> functionality will probably Just Work with older QEMUs.
> 
> > I think it would also be
> > useful if you could do the same for a couple DSDT entries (eg,
> > \_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
> > guest build the AML from that info.
> 
> Likewise the slot count should be available too.  We hard code slots
> today but it is something we should model properly.  We would model it
> using QOM of course.
> 
> Internally within QEMU, this initial discussion started by saying that
> any ACPI generation within QEMU should happen strictly with QOM
> methods.  This was the crux of my argument, if QOM is the only interface
> we need, everything is there for the firmware to do the same job that
> QEMU would be doing.

I think we keep talking past each other.  You seem to be pointing out
that the information seabios uses to patch its hardcoded tables can be
passed in via QOM.  Agreed, it can.  I'm pointing out all the info
that is hardcoded in seabios - I don't see how that can be passed via
QOM.

All the hardcoded data in seabios is a problem, because when it
changes (and it frequently does) it requires changes to both QEMU and
SeaBIOS and those changes have to be coordinated.  The key reason for
moving the tables into QEMU is not that it can better patch the tables
- the advantage is that it can hardcode the tables that need patching.

I can cite several recent examples of seabios change requests that
require changing of the hardcoded tables: liguang wants to add an
"embedded controller" hardware device which requires changes to
seabios' dsdt, Corey Minyard wants to add IPMI hardware support (both
smbios changes and DSDT changes), and Corey Bryant wants to add TPM
hardware support.

How do we solve the problem of seabios having a ton of hardcoded
information about the qemu hardware, and seabios having to change with
the hardware modifications that qemu makes?

-Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

David Woodhouse  writes:

> On Mon, 2013-06-10 at 18:34 -0500, Anthony Liguori wrote:
>> Internally within QEMU, this initial discussion started by saying that
>> any ACPI generation within QEMU should happen strictly with QOM
>> methods.  This was the crux of my argument, if QOM is the only
>> interface we need, everything is there for the firmware to do the same
>> job that QEMU would be doing.
>
> That's nice in theory, but I'm not sure how it works as things evolve
> and new things / new features get exposed. The firmware's
> *interpretation* of the QOM tree needs to be kept in sync with qemu.
>
> Hm, make that: The firmwares' *interpretation*…
>
> Let's take a specific, recent example. We fixed the PIIX4 code to
> actually handle the hard reset on port 0xcf9. We need to fix the ACPI
> tables to indicate a usable RESET_REG.

Very good example.

Normally, we try to be bug-for-bug compatible for guests such that -M
pc-1.4 would behave exactly the same.

In this case, we failed to introduce a property to control this behavior
like we should have.  If this changes the guest ACPI tables, it
definitely should definitely be set based on a property.

This is a good example of why this approach is good for QEMU, it helps
us catch stuff like this.

> How is that exposed in the QOM tree, and how does it all work? With qemu
> exposing ACPI tables in their close-to-final form, it's just fine. Boot
> with a recent qemu and it's all nice and shiny; boot with an old qemu
> and it doesn't reset properly.

If we did this right in QEMU, we'd have to introduce a QOM property
anyway as that's how we trigger differences in machine behavior.  And -M
pc-1.4 ought to generate the same tables as qemu 1.4 did.

Regards,

Anthony Liguori

> But if the firmware has to be updated to interpret the new feature
> advertised in the QOM tree and translate it into the ACPI table, then we
> haven't really got much of an improvement.
>
> Please explain how this is supposed to work in *practice*.
>
> -- 
> dwmw2

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[David Woodhouse]

On Mon, 2013-06-10 at 18:34 -0500, Anthony Liguori wrote:
> Internally within QEMU, this initial discussion started by saying that
> any ACPI generation within QEMU should happen strictly with QOM
> methods.  This was the crux of my argument, if QOM is the only
> interface we need, everything is there for the firmware to do the same
> job that QEMU would be doing.

That's nice in theory, but I'm not sure how it works as things evolve
and new things / new features get exposed. The firmware's
*interpretation* of the QOM tree needs to be kept in sync with qemu.

Hm, make that: The firmwares' *interpretation*…

Let's take a specific, recent example. We fixed the PIIX4 code to
actually handle the hard reset on port 0xcf9. We need to fix the ACPI
tables to indicate a usable RESET_REG.

How is that exposed in the QOM tree, and how does it all work? With qemu
exposing ACPI tables in their close-to-final form, it's just fine. Boot
with a recent qemu and it's all nice and shiny; boot with an old qemu
and it doesn't reset properly.

But if the firmware has to be updated to interpret the new feature
advertised in the QOM tree and translate it into the ACPI table, then we
haven't really got much of an improvement.

Please explain how this is supposed to work in *practice*.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Kevin O'Connor  writes:

> On Mon, Jun 10, 2013 at 04:45:53PM -0500, Anthony Liguori wrote:
>> This discussion comes down to two things I think: (a) our existing
>> firmware interface is pretty poor (b) we are duplicating work because of
>> firmware licensing.
>> 
>> We can fix (a) and there's lots of value in doing that in terms of
>> improving support for other architectures.  We've discussed (b) in other
>> threads and I've stated my opinion on the direction we need to take.
>
> I'm not concerned about (b).
>
> I'm quite curious how you are planning to solve (a).  I think it would
> help move this conversation forward if you could take a couple acpi
> tables in use today (eg, madt, srat) and describe the future format
> and location for each field in those tables.

Sure.  Today we expose the device model through a graph that can be
losely mapped to a filesystem.  We expose two methods of interest as
part of our management api:

{ 'type': 'ObjectPropertyInfo',
  'data': { 'name': 'str', 'type': 'str' } }

{ 'command': 'qom-list',
  'data': { 'path': 'str' },
  'returns': [ 'ObjectPropertyInfo' ] }

{ 'command': 'qom-get',
  'data': { 'path': 'str', 'property': 'str' },
  'returns': 'visitor',
  'gen': 'no' }

I am proposing that we replace fw_cfg with a device that exports the QOM
tree (read-only) via firmware.

We have a utility called qom-fuse that can mount a fuse filesystem that
exports this as a proper filesystem too.

Here's a small example:

[06:26 PM] anthony🐵 titi:~/git/qemu/QMP/tmp$ find
.
./machine
./machine/i440fx
./machine/i440fx/ioapic
./machine/i440fx/ioapic/parent_bus
./machine/i440fx/ioapic/realized
./machine/i440fx/ioapic/type
./machine/i440fx/pci.0
./machine/i440fx/pci.0/child[5]
./machine/i440fx/pci.0/child[4]
./machine/i440fx/pci.0/child[3]
./machine/i440fx/pci.0/child[2]
./machine/i440fx/pci.0/child[1]
./machine/i440fx/pci.0/child[0]
...

For the MADT table, right now SeaBIOS needs the CPU count.  That can be
found by counting the number of CPU nodes.  Today cpus are unattached so
they appear in /machine/unattached but we should put them in a
/machine/cpu container for clarity.

Presumably it would be good to expose the apic id too.  This is already
in the QOM tree.

QOM is the full representation of the device state so we should not ever
need to add additional things to fw_cfg.  More likely than not, when
SeaBIOS needs more information, it's already there so added
functionality will probably Just Work with older QEMUs.

> I think it would also be
> useful if you could do the same for a couple DSDT entries (eg,
> \_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
> guest build the AML from that info.

Likewise the slot count should be available too.  We hard code slots
today but it is something we should model properly.  We would model it
using QOM of course.

Internally within QEMU, this initial discussion started by saying that
any ACPI generation within QEMU should happen strictly with QOM
methods.  This was the crux of my argument, if QOM is the only interface
we need, everything is there for the firmware to do the same job that
QEMU would be doing.

Regards,

Anthony Liguori

>
> -Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Kevin O'Connor]

On Mon, Jun 10, 2013 at 04:45:53PM -0500, Anthony Liguori wrote:
> This discussion comes down to two things I think: (a) our existing
> firmware interface is pretty poor (b) we are duplicating work because of
> firmware licensing.
> 
> We can fix (a) and there's lots of value in doing that in terms of
> improving support for other architectures.  We've discussed (b) in other
> threads and I've stated my opinion on the direction we need to take.

I'm not concerned about (b).

I'm quite curious how you are planning to solve (a).  I think it would
help move this conversation forward if you could take a couple acpi
tables in use today (eg, madt, srat) and describe the future format
and location for each field in those tables.  I think it would also be
useful if you could do the same for a couple DSDT entries (eg,
\_SB.PCI0, \_SB.PCI0.ISA) and also describe how you plan to have the
guest build the AML from that info.

-Kevin

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Laszlo Ersek  writes:

> On 06/10/13 21:57, Anthony Liguori wrote:
>> Laszlo Ersek  writes:
>
>> 
>> I don't understand this piece.
>> 
>> Other than TianoCore being a weird environment, what made this more
>> difficult than it is to generate the tables in QEMU?
>
> QEMU can pass down two kinds of SMBIOS config items over fw_cfg:
> verbatim tables and individual fields.
>
> The verbatim table kind means "install this table as-is". I'm not sure
> how SeaBIOS solves it, in OVMF you call Smbios->Add().
>
> Regarding the "field kind" config item: for each table type required by
> the SMBIOS spec, if qemu doesn't provide such a table in whole / in
> verbatim, then the firmware must install a default table for that type,
> *and* patch it with any referring field kind config items.
>
> Given the current format of the "field kind" config item, it is
> currently not possible to just iterate over the "field kind" config
> items, and patch whatever field in whatever table they refer to. The
> information contained in the "field kind" config item is insufficient to
> do this.
>
> The "field kind" config item designates a (table, field offset) pair. If
> that offset points at a string-typed SMBIOS field (basically string
> serial number into the unformatted area), one must update the
> unformatted string area after the actual table fields. (In OVMF one does
> this with a different call, Smbios->UpdateStrings(), and at that point
> the table even must have been installed already.)
>
> If the offset points at a non-string-typed (= in-formatted-area) field,
> the field must be overwritten in-place. (In OVMF this must happen
> *before* table installation, ie. before Smbios->Add().)

Okay, I understand the problem here.  Thanks for the explanation.  I had
assumed that the smbios_field structure was the binary representation of
the table.  I didn't realize it was our own format used to pass
information over.

> So, the current fw_cfg representation is insufficient, and the firmware
> must extend (or qemu should have extended) the info contents with this
> distinction between formatted and unformatted. Again this differs from
> field to field and the origin of that classification is the SMBIOS spec.
>
> Until this point in my email there has been no clear indication whether
> qemu or the firmware should do this. *Some* side will have to suffer
> shoveling the field/offset info, and a table template for each type,
> from the SMBIOS spec into code. I described the above solely as
> background, to set the landscape.
>
> (a) The first thing that tips the scale is: of qemu there is one, and of
> firmware, there exist at least two, *wildly different* implementations.
> (Apologies, I forgot about coreboot; not sure if it's affected.) It's
> not a design purity argument, just what's cheaper.

The only reason there are two implementations is a licensing problem
that must be solved anyway.

> (b) The second thing that further unbalances the scale: suppose you
> decide to add further field-kind overrides to qemu. For example,
> currently Type 3 (chassis information, table required by SMBIOS spec)
> cannot be overridden field-wise.

The reason we expose blobs in SMBIOS is that blobs can be vendor
specific and there was a strong desire to allow vendor specific blobs to
be passed through.  There really is no choice but to expose a blob
interface for that.

> If a user is unhappy with her firmware's default Type 3 table, she's
> allowed to hex-edit a Type 3 blob, and pass it with "-smbios
> file=type3.blob" on the qemu command line. If you as developer want to
> provide her with more flexibility, you need to
> (i) patch qemu (of course), to recognize individual fields on the
> command line, and to populate fw_cfg with them,
> (ii) patch *every single firmware* supported by qemu to *look for* those
> (type=3,offset) field-kind config entries, and to effectuate them.
>
> If you pass down only verbatim tables, then (ii) falls away. You don't
> have to update SeaBIOS, nor OVMF.

OVMF is proprietary.  It is not "supported" by QEMU.  Firmware is a
fundamental part of our stack.  I'm not really convinced that
QEMU<->firmware is a GPL boundary because of how tightly the two are
linked.

Moving large chunks of firmware code into QEMU just to avoid solving
licensing issues is a non-starter with me.

> As a bonus, you don't have to care
> about versions: age-old firmware that only knows how to install verbatim
> tables will immediately benefit from qemu's new-fangled Type 3
> flexibility.

This presume that (1) you only care about exposing this information to
x86 guests (2) that the firmware will never have any need to patch these
blobs.

> Case in point: .
> (Non-public bug, sorry about that. I think I can disclose that it is
> about supporting the "field kind" config entry for Type 3 tables.)

This information is *exactly* the kind of information that you want to
be exposed across architectures.  I

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

Sorry about replying to myself...

On 06/10/13 22:43, Laszlo Ersek wrote:

> It's not a design purity argument, just what's cheaper.

You probably consider SMBIOS/ACPI table generation "guest code" that has
no place in the machine emulator. You're likely willing to expose the
needed bits on a case-by-case basis, in their barest representation.

I must consider the entire stack, from the libvirt top, down to the
guest kernel or root shell, and see where it is cheapest to effect
changes. The higher I can hoist something the better.

Modulo, it depends on what element of the stack people think of as
central / indispensable -- that element should support the feature directly.

(Returning to my earlier example, it would be theoretically viable *I
guess* to equip libvirt to pre-format a Type 3 SMBIOS blob in a file,
and pass it with "-smbios file=...", but many people don't use libvirt.
It also might not apply to other emulators supported by libvirt.)

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

On 06/10/13 21:57, Anthony Liguori wrote:
> Laszlo Ersek  writes:

>> Please take a look at my series for OVMF that adds basic support for
>> SMBIOS tables. It took me three days of basically uninterrupted coding
>> and two approaches to throw away until I got something submittable (with
>> default tables for only type 0 and type 1).
>>
>> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/3037
>>
>> I don't want to invite accusations like "this is a strawman argument,
>> noone was speaking about SMBIOS", but the selective, dynamic patching is
>> somewhat similar between AML and SMBIOS in any given boot firmware. You
>> got a big bunch of named offsets that must be mangled individually.
>>
>> Allowing the firmware to only install blobs verbatim, or maybe patch
>> them but only in a completely programmatic manner (ie. without specific
>> field names & offsets in the firmware, which I did try for SMBIOS in
>> OVMF, and failed, (*)), would be a big help.
>>
>> ((*) because the fw_cfg format of individual SMBIOS fields doesn't
>> distinguish formatted fields from strings in the unformatted area)
> 
> I don't understand this piece.
> 
> Other than TianoCore being a weird environment, what made this more
> difficult than it is to generate the tables in QEMU?

QEMU can pass down two kinds of SMBIOS config items over fw_cfg:
verbatim tables and individual fields.

The verbatim table kind means "install this table as-is". I'm not sure
how SeaBIOS solves it, in OVMF you call Smbios->Add().

Regarding the "field kind" config item: for each table type required by
the SMBIOS spec, if qemu doesn't provide such a table in whole / in
verbatim, then the firmware must install a default table for that type,
*and* patch it with any referring field kind config items.

Given the current format of the "field kind" config item, it is
currently not possible to just iterate over the "field kind" config
items, and patch whatever field in whatever table they refer to. The
information contained in the "field kind" config item is insufficient to
do this.

The "field kind" config item designates a (table, field offset) pair. If
that offset points at a string-typed SMBIOS field (basically string
serial number into the unformatted area), one must update the
unformatted string area after the actual table fields. (In OVMF one does
this with a different call, Smbios->UpdateStrings(), and at that point
the table even must have been installed already.)

If the offset points at a non-string-typed (= in-formatted-area) field,
the field must be overwritten in-place. (In OVMF this must happen
*before* table installation, ie. before Smbios->Add().)

So, the current fw_cfg representation is insufficient, and the firmware
must extend (or qemu should have extended) the info contents with this
distinction between formatted and unformatted. Again this differs from
field to field and the origin of that classification is the SMBIOS spec.

Until this point in my email there has been no clear indication whether
qemu or the firmware should do this. *Some* side will have to suffer
shoveling the field/offset info, and a table template for each type,
from the SMBIOS spec into code. I described the above solely as
background, to set the landscape.

(a) The first thing that tips the scale is: of qemu there is one, and of
firmware, there exist at least two, *wildly different* implementations.
(Apologies, I forgot about coreboot; not sure if it's affected.) It's
not a design purity argument, just what's cheaper.

(b) The second thing that further unbalances the scale: suppose you
decide to add further field-kind overrides to qemu. For example,
currently Type 3 (chassis information, table required by SMBIOS spec)
cannot be overridden field-wise.

If a user is unhappy with her firmware's default Type 3 table, she's
allowed to hex-edit a Type 3 blob, and pass it with "-smbios
file=type3.blob" on the qemu command line. If you as developer want to
provide her with more flexibility, you need to
(i) patch qemu (of course), to recognize individual fields on the
command line, and to populate fw_cfg with them,
(ii) patch *every single firmware* supported by qemu to *look for* those
(type=3,offset) field-kind config entries, and to effectuate them.

If you pass down only verbatim tables, then (ii) falls away. You don't
have to update SeaBIOS, nor OVMF. As a bonus, you don't have to care
about versions: age-old firmware that only knows how to install verbatim
tables will immediately benefit from qemu's new-fangled Type 3 flexibility.

Case in point: .
(Non-public bug, sorry about that. I think I can disclose that it is
about supporting the "field kind" config entry for Type 3 tables.)


For ACPI tables, especially for tables containing AML, multiply the cost
of (ii) by twenty. Many more offsets to patch and vastly more
dependencies via fw_cfg.

Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Laszlo Ersek  writes:

> On 06/10/13 20:58, Anthony Liguori wrote:
>> "Michael S. Tsirkin"  writes:
>> 
>>> This adds support for device hotplug behind
>>> pci bridges. Bridge devices themselves need
>>> to be pre-configured on qemu command line.
>>>
>>> One of the goals of this project was to
>>> demonstrate the advantages of generating
>>> ACPI tables in QEMU.
>>> In my opinion, it does this successfully.
>> 
>> Since you've gone out of your way to make this difficult to actually
>> review...
>
> I haven't tried to review the patch yet, but why would you say such a
> thing? Michael wants to convince you. There's no way he could sneak past
> you in this patch. You surely won't say "it's too hard to review, I'll
> yield", and he's obviously aware.

The patch is very large and depends on another large series.  It's not
split up as a logical series of changes and more importantly doesn't
solve significant problems (like live migration).

It makes it very difficult to properly evaluate the approach.  But even
taking those sort cuts, it looks like a lot of code in QEMU to avoid
what appears to be a significantly smaller amount of code in SeaBIOS.

There isn't deep dependency on information only available in QEMU.
AFAICT the only advantage of being in QEMU is being able to use a couple
data types from glib.

> [snip]
>
>>>* No need for yet another interface to bios to detect qemu version
>>>  to check if it's safe to activate new code,
>>>  and to ship multiple table versions:
>> 
>> We only care about supporting one version of SeaBIOS.  SeaBIOS should
>> only care about supporting one version of QEMU.  We're not asking it to
>> support multiple versions.
>
> Kevin requires cross-compat between {old, new} qemu and {old, new} SeaBIOS:
>
> http://thread.gmane.org/gmane.comp.emulators.qemu/202005/focus=202072
>
>>>  we generated code so we know this is new qemu
>>>* Use of glib's GArray makes it much easier to build
>>>  up tables in code without need for iasl and code patching
>> 
>> Adding a dynamic array to SeaBIOS isn't rocket science.
>
> Please take a look at my series for OVMF that adds basic support for
> SMBIOS tables. It took me three days of basically uninterrupted coding
> and two approaches to throw away until I got something submittable (with
> default tables for only type 0 and type 1).
>
> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/3037
>
> I don't want to invite accusations like "this is a strawman argument,
> noone was speaking about SMBIOS", but the selective, dynamic patching is
> somewhat similar between AML and SMBIOS in any given boot firmware. You
> got a big bunch of named offsets that must be mangled individually.
>
> Allowing the firmware to only install blobs verbatim, or maybe patch
> them but only in a completely programmatic manner (ie. without specific
> field names & offsets in the firmware, which I did try for SMBIOS in
> OVMF, and failed, (*)), would be a big help.
>
> ((*) because the fw_cfg format of individual SMBIOS fields doesn't
> distinguish formatted fields from strings in the unformatted area)

I don't understand this piece.

Other than TianoCore being a weird environment, what made this more
difficult than it is to generate the tables in QEMU?

Regards,

Anthony Liguori

>
> Not rocket science, just churn and busywork.
>
> [snip]
>
>> So TL;DR, you break a bunch of stuff and introduce a mess of code.  It
>> would be less code and wouldn't break anything to add this logic to
>> SeaBIOS.
>> 
>> How is this even a discussion?
>
> Well it isn't for me any more; count me out. I'll go with the flow.
>
> Cheers,
> Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

Peter Maydell  writes:

> On 10 June 2013 19:58, Anthony Liguori  wrote:
>> We only care about supporting one version of SeaBIOS.  SeaBIOS should
>> only care about supporting one version of QEMU.  We're not asking it to
>> support multiple versions.
>
> I'm confused, how does this work in cross-version migration?
> The BIOS is part of the guest, so I'd expect QEMU would
> have to support whatever it happens to be. (Plus how do
> you enforce the version-dependency?)

The BIOS is migrated but reset upon reboot.

Regards,

Anthony Liguori

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Laszlo Ersek]

On 06/10/13 20:58, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
>> This adds support for device hotplug behind
>> pci bridges. Bridge devices themselves need
>> to be pre-configured on qemu command line.
>>
>> One of the goals of this project was to
>> demonstrate the advantages of generating
>> ACPI tables in QEMU.
>> In my opinion, it does this successfully.
> 
> Since you've gone out of your way to make this difficult to actually
> review...

I haven't tried to review the patch yet, but why would you say such a
thing? Michael wants to convince you. There's no way he could sneak past
you in this patch. You surely won't say "it's too hard to review, I'll
yield", and he's obviously aware.

[snip]

>>* No need for yet another interface to bios to detect qemu version
>>  to check if it's safe to activate new code,
>>  and to ship multiple table versions:
> 
> We only care about supporting one version of SeaBIOS.  SeaBIOS should
> only care about supporting one version of QEMU.  We're not asking it to
> support multiple versions.

Kevin requires cross-compat between {old, new} qemu and {old, new} SeaBIOS:

http://thread.gmane.org/gmane.comp.emulators.qemu/202005/focus=202072

>>  we generated code so we know this is new qemu
>>* Use of glib's GArray makes it much easier to build
>>  up tables in code without need for iasl and code patching
> 
> Adding a dynamic array to SeaBIOS isn't rocket science.

Please take a look at my series for OVMF that adds basic support for
SMBIOS tables. It took me three days of basically uninterrupted coding
and two approaches to throw away until I got something submittable (with
default tables for only type 0 and type 1).

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/3037

I don't want to invite accusations like "this is a strawman argument,
noone was speaking about SMBIOS", but the selective, dynamic patching is
somewhat similar between AML and SMBIOS in any given boot firmware. You
got a big bunch of named offsets that must be mangled individually.

Allowing the firmware to only install blobs verbatim, or maybe patch
them but only in a completely programmatic manner (ie. without specific
field names & offsets in the firmware, which I did try for SMBIOS in
OVMF, and failed, (*)), would be a big help.

((*) because the fw_cfg format of individual SMBIOS fields doesn't
distinguish formatted fields from strings in the unformatted area)

Not rocket science, just churn and busywork.

[snip]

> So TL;DR, you break a bunch of stuff and introduce a mess of code.  It
> would be less code and wouldn't break anything to add this logic to
> SeaBIOS.
> 
> How is this even a discussion?

Well it isn't for me any more; count me out. I'll go with the flow.

Cheers,
Laszlo

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Peter Maydell]

On 10 June 2013 19:58, Anthony Liguori  wrote:
> We only care about supporting one version of SeaBIOS.  SeaBIOS should
> only care about supporting one version of QEMU.  We're not asking it to
> support multiple versions.

I'm confused, how does this work in cross-version migration?
The BIOS is part of the guest, so I'd expect QEMU would
have to support whatever it happens to be. (Plus how do
you enforce the version-dependency?)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu: piix: PCI bridge ACPI hotplug support

[Anthony Liguori]

"Michael S. Tsirkin"  writes:

> This adds support for device hotplug behind
> pci bridges. Bridge devices themselves need
> to be pre-configured on qemu command line.
>
> One of the goals of this project was to
> demonstrate the advantages of generating
> ACPI tables in QEMU.
> In my opinion, it does this successfully.

Since you've gone out of your way to make this difficult to actually
review...

>
>* This saves the need to provide a new interface for firmware
>  to discover bus number to pci brige mapping

Do you mean fw_cfg?  The BIOS normally does bus numbering.  I see no
reason for it not to.

>* No need for yet another interface to bios to detect qemu version
>  to check if it's safe to activate new code,
>  and to ship multiple table versions:

We only care about supporting one version of SeaBIOS.  SeaBIOS should
only care about supporting one version of QEMU.  We're not asking it to
support multiple versions.

>  we generated code so we know this is new qemu
>* Use of glib's GArray makes it much easier to build
>  up tables in code without need for iasl and code patching

Adding a dynamic array to SeaBIOS isn't rocket science.

>
> I have also tried to address the concerns that
> Anthony has raised with this approach, please see below.
>
> Design:
> - each bus gets assigned a number 0-255
> - generated ACPI code writes this number
>   to a new BSEL register, then uses existing
>   UP/DOWN registers to probe slot status;
>   to eject, write number to BSEL register,
>   then slot into existing EJ
>
> This is to address the ACPI spec requirement to
> avoid config cycle access to any bus except PCI roots.
>
> Portability:
> - Non x86 (or any Linux) platforms don't need any of this code.
>   They can keep happily using SHPC the way
>   they always did.
>
> Things to note:
> - this is on top of acpi patchset posted a month ago,
>   with a small patch adding a core function to walk all
>   pci buses, on top.
>   Can also be found in my git tree
> git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi
>
> - Extensive use of glib completely removes
>   pointer math in new code: we use
>   g_array_append_vals exclusively.
>   There's no code patching in new code.
>   This is in response to comments about security
>   concerns with adding code to qemu.
>   In this sense it is more secure than existing
>   code in hw/acpi/core.c - that can be switched to
>   glib if desired.
>
> - New code (not imported from seabios)
>   uses glib to reduce dependency on iasl.
>   With time all code can be rewritted to
>   use glib and avoid iasl, if desired.
>
> - As was the case previously,
>   systems that lack working iasl are detected at configure time,
>   pre-generated hex files in source tree are used in this case.
>   This addresses the concern with iasl/big-endian
>   systems.
>
> - Compile tested only. Migration is known to be
>   broken - there are a bunch of TODO tags in code.
>   It was agreed on the last qemu conf meeting that
>   this code is posted without looking at migration,
>   with understanding that it is addressed before
>   being merged. Please do not mistake this
>   limitation for a fundamental one - I have a
>   very good idea how to fix it, including
>   cross-version migration.
>
> - Cross version migration: when running with -M 1.5
>   and older, all ACPI table generation should be disabled.
>   We'll present FW_CFG interface compatible with 1.5.

So TL;DR, you break a bunch of stuff and introduce a mess of code.  It
would be less code and wouldn't break anything to add this logic to
SeaBIOS.

How is this even a discussion?

Regards,

Anthony Liguori

>
> Cc: Jordan Justen 
> Cc: Anthony Liguori 
> Cc: Laszlo Ersek 
> Cc: Gerd Hoffmann 
> Cc: seab...@seabios.org
> Cc: David Woodhouse 
> Cc: Kevin O'Connor 
> Cc: Peter Maydell 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  docs/specs/acpi_pci_hotplug.txt  |   8 +
>  include/hw/i386/pc.h |   4 +-
>  hw/i386/acpi-dsdt.dsl|  36 +++-
>  hw/i386/ssdt-pcihp.dsl   |  51 -
>  hw/acpi/piix4.c  | 145 ++
>  hw/i386/acpi-build.c | 411 
> ++-
>  hw/i386/Makefile.objs|   2 +-
>  hw/i386/ssdt-pcihp.hex.generated | 108 --
>  8 files changed, 510 insertions(+), 255 deletions(-)
>  delete mode 100644 hw/i386/ssdt-pcihp.dsl
>  delete mode 100644 hw/i386/ssdt-pcihp.hex.generated
>
> diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
> index a839434..951b99a 100644
> --- a/docs/specs/acpi_pci_hotplug.txt
> +++ b/docs/specs/acpi_pci_hotplug.txt
> @@ -43,3 +43,11 @@ PCI removability status (IO port 0xae0c-0xae0f, 4-byte 
> access):
>  
>  Used by ACPI BIOS _RMV m