PCI Hotplug ACPI device names only 3 characters long

PCI Hotplug ACPI device names only 3 characters long

[Marcello Sylverster Bauer]

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

Re: PCI Hotplug ACPI device names only 3 characters long

[Philippe Mathieu-Daudé]

Hi Marcello,

On 5/9/23 17:05, 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)
> ```

What do you mean by "before" / "after"?

> 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
> 

Re: PCI Hotplug ACPI device names only 3 characters long

[Marcello Sylverster Bauer]

Hi Philippe,

On 9/5/23 17:09, Philippe Mathieu-Daudé wrote:
> Hi Marcello,
> 
> On 5/9/23 17:05, 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)
>> ```
> 
> What do you mean by "before" / "after"?

Before is what is written in my SSDT ASL source file that is provided to 
QEMU via the "-acpitable" flag. After is what is actually written to the 
SSDT inside the VM.

If you compile and decompile the source file with iasl, you will get the 
same result.

> 
>> 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
>>
> 

Re: PCI Hotplug ACPI device names only 3 characters long

[Michael S. Tsirkin]

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. 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.
2.  You can probably add something to existing ACPI devices using Scope().
    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.


-- 
MST

Re: PCI Hotplug ACPI device names only 3 characters long

[Marcello Sylverster Bauer]

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.

> 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 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

> 
> 

Re: PCI Hotplug ACPI device names only 3 characters long

[Michael S. Tsirkin]

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.

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.

> 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
> 
> > 
> > 

Re: PCI Hotplug ACPI device names only 3 characters long

[Igor Mammedov]

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.
> 
> > > 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
> >   
> > > 
> > >   
> 

Re: PCI Hotplug ACPI device names only 3 characters long

[Marcello Sylverster Bauer]

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
>>>    
>>>>
>>>>    
>>
>