On Tue, 5 Sep 2023 16:43:54 -0400
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Tue, Sep 05, 2023 at 07:45:12PM +0200, Marcello Sylverster Bauer wrote:
> > Hi Michael,
> >
> > On 9/5/23 18:44, Michael S. Tsirkin wrote:
> > > On Tue, Sep 05, 2023 at 05:05:33PM +0200, Marcello Sylverster Bauer
> > > wrote:
> > > > Greetings,
> > > >
> > > > I'm currently working on a project to support Intel IPU6 in QEMU via
> > > > VFIO so
> > > > that the guest system can access the camera. This requires extending the
> > > > ACPI device definition so that the guest knows how to access the camera.
> > > >
> > > > However, I cannot extend the PCI devices because their names are not 4
> > > > characters long and therefore do not follow the ACPI specification.
> > > >
> > > > When I use '-acpitable' to include my own SSDT for the IPU6 PCI device,
> > > > it
> > > > does not allow me to declare the device as an External Object because it
> > > > automatically adds padding underscores.
> > > >
> > > > e.g.
> > > > Before:
> > > > ```
> > > > External(_SB.PCI0.S18.SA0, DeviceObj)
> > > > ```
> > > > After:
> > > > ```
> > > > External(_SB.PCI0.S18_.SA0_, DeviceObj)
> > > > ```
> > > >
> > > > Adding the underscore padding is hard coded in iASL and also in QEMU
> > > > when
> > > > parsing an ASL file. (see: build_append_nameseg())
> > > >
> > > > So here are my questions:
> > > > 1. Is there a solution to extend the ACPI PCI device using '-acpitable'
> > > > without having to patch iASL or QEMU?
> > > > 2. Are there any plans to change the names to comply with the ACPI spec?
> > > > (e.g. use "S%.03X" format string instead)
> > > >
> > > > Thanks
> > > > Marcello
> > >
> > >
> > > 1. All names in ACPI are always exactly 4 characters long. _ is a legal
> > > character
> > > but names beginning with _ are reserved.
> >
> > Exactly, which is why I want to address this issue here. Currently, Qemu
> > generates ACPI device names with only 3 characters. (See
> > build_append_pci_bus_devices() in hw/i386/acpi-build.c).
> > For example, the device I want to append entries to has the path
> > "_SB.PCI0.S18.SA0", but I can't because of the two auto-generated devices
> > with only 3 characters in their names.
if your ssdt really has shorter names. You can use iasl to de-compile it
wiht bytte code included and check manually if if namesegment(s) are short
or padded with '_'.
(ex: iasl -d -l DSDT)
>
> They are 4 characters otherwise OSPMs wouldn't work.
> In your example the path is _SB.PCI0.S18_.SA0_ - you disassembler probably
> just helpfully hides it for readability.
>
> > > There's no rule in ACPI
> > > spec that says they need to follow S%.03X or any other specific
> > > format.
> > > I'm pretty sure we do follow the ACPI specification in this but feel
> > > free to
> > > prove me wrong.
> >
> > You have misunderstood me. Currently, Qemu uses the following format to
> > create PCI ACPI devices:
> >
> > ```
> > aml_name("S%.02X", devfn)
> > ```
> >
> > My question is whether we should change it to something that results in a 4
> > character name like "S%.03X" or "S%.02X_".
>
> I think you misunderstand the code. Look at build_append_nameseg and you will
> see that the name is always ACPI_NAMESEG_LEN characters which equals 4.
ACPI 6.5 spec says this:
20.2.2 Name Objects Encoding
...
NameSeg :=
...
// Notice that NameSegs shorter than 4 characters are filled with trailing
underscores (‘_’s).
that's what QEMU does if user has supplied shorter than 4 characters name
>
> > I have tested it and it works fine as long as any hardcoded path references
> > are adjusted. But I'm not 100% sure if this could cause any regressions.
> >
> > > 2. You can probably add something to existing ACPI devices using
> > > Scope().
> >
> > I'm pretty sure the external object is required when loading a separate
> > SSDT, but I'll try by just using scopes.
> >
> > > I would not advise relying on this - current names are not a stable
> > > interface that we guarantee across QEMU versions.
> > > If adding this functionality is desirable, I think we'll need some
> > > new interface
> > > to set a stable ACPI name. Maybe using aliases.
> >
> > Currently I'm just working on a PoW to get IPU6 working in QEMU, so
> > instability is fine.
> >
> > Thanks,
> > Marcello
> >
> > >
> > >
>
