qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PCI Hotplug ACPI device names only 3 characters long

From: Marcello Sylverster Bauer
Subject: Re: PCI Hotplug ACPI device names only 3 characters long
Date: Wed, 6 Sep 2023 12:45:05 +0200 
Hi Igor, Hi Michael,

On 9/6/23 10:19, Igor Mammedov wrote:

On Tue, 5 Sep 2023 16:43:54 -0400
"Michael S. Tsirkin" <mst@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.
Oh my mistake, looks like Michael is right and the padding underscores are just hidden in the decompiled ACPI tables and in the Linux kernel ACPI bus_id, so I jumped to the wrong conclusion: 

```
                Device (SA0)
                {

    2824: 5B 82 4F 04 53 41 30 5F                          // [.O.SA0_
```

dmesg
```
[    0.837141] DEBUG: adev=0000000073b4ccad bid="SA0"
```




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
Yes, that's what I meant, but it looks like I didn't understand the QEMU ACPI code properly. 

Thanks for the clarification and sorry for the noise.

Marcello




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
reply via email to
[Prev in Thread] Current Thread [Next in Thread]