[PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table

Alireza Sanaee via posted 5 patches 6 months, 3 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>
There is a newer version of this series
[PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Alireza Sanaee via 6 months, 3 weeks ago
From: Yicong Yang <yangyicong@hisilicon.com>

The lastest ACPI spec 6.5 support PPTT revision 3. Update it
by handy. This is compatible with previous revision.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 hw/acpi/aml-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 3010325ca423..e5401dfdb1a8 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
     uint32_t pptt_start = table_data->len;
     uint32_t root_offset;
     int n;
-    AcpiTable table = { .sig = "PPTT", .rev = 2,
+    AcpiTable table = { .sig = "PPTT", .rev = 3,
                         .oem_id = oem_id, .oem_table_id = oem_table_id };
 
     acpi_table_begin(&table, table_data);
-- 
2.34.1
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Michael S. Tsirkin 6 months, 3 weeks ago
On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
> by handy. This is compatible with previous revision.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>


I don't get it. Why are you updating it? Which features
from the new one are you using?

> ---
>  hw/acpi/aml-build.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 3010325ca423..e5401dfdb1a8 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>      uint32_t pptt_start = table_data->len;
>      uint32_t root_offset;
>      int n;
> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
>  
>      acpi_table_begin(&table, table_data);
> -- 
> 2.34.1
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Yicong Yang via 6 months, 3 weeks ago
On 2025/4/23 20:39, Michael S. Tsirkin wrote:
> On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
>> by handy. This is compatible with previous revision.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> 
> 
> I don't get it. Why are you updating it? Which features
> from the new one are you using?
> 

no new features for this patchset. considered updating it to the latest ACPI
spec since we're going to touch the PPTT table and tested data.

>> ---
>>  hw/acpi/aml-build.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 3010325ca423..e5401dfdb1a8 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>>      uint32_t pptt_start = table_data->len;
>>      uint32_t root_offset;
>>      int n;
>> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
>> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
>>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
>>  
>>      acpi_table_begin(&table, table_data);
>> -- 
>> 2.34.1
> 
> 
> .
>
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Michael S. Tsirkin 6 months, 3 weeks ago
On Wed, Apr 23, 2025 at 10:15:42PM +0800, Yicong Yang wrote:
> On 2025/4/23 20:39, Michael S. Tsirkin wrote:
> > On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee wrote:
> >> From: Yicong Yang <yangyicong@hisilicon.com>
> >>
> >> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
> >> by handy. This is compatible with previous revision.
> >>
> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> > 
> > 
> > I don't get it. Why are you updating it? Which features
> > from the new one are you using?
> > 
> 
> no new features for this patchset. considered updating it to the latest ACPI
> spec since we're going to touch the PPTT table and tested data.

it's best to wait until there are actual features you need.
don't make changes for the sake of changes, there's always
some risk.

> >> ---
> >>  hw/acpi/aml-build.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >> index 3010325ca423..e5401dfdb1a8 100644
> >> --- a/hw/acpi/aml-build.c
> >> +++ b/hw/acpi/aml-build.c
> >> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> >>      uint32_t pptt_start = table_data->len;
> >>      uint32_t root_offset;
> >>      int n;
> >> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
> >> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
> >>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
> >>  
> >>      acpi_table_begin(&table, table_data);
> >> -- 
> >> 2.34.1
> > 
> > 
> > .
> >
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Jonathan Cameron via 6 months, 3 weeks ago
On Wed, 23 Apr 2025 11:35:46 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Apr 23, 2025 at 10:15:42PM +0800, Yicong Yang wrote:
> > On 2025/4/23 20:39, Michael S. Tsirkin wrote:  
> > > On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee wrote:  
> > >> From: Yicong Yang <yangyicong@hisilicon.com>
> > >>
> > >> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
> > >> by handy. This is compatible with previous revision.
> > >>
> > >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > >> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>  
> > > 
> > > 
> > > I don't get it. Why are you updating it? Which features
> > > from the new one are you using?
> > >   
> > 
> > no new features for this patchset. considered updating it to the latest ACPI
> > spec since we're going to touch the PPTT table and tested data.  
> 
> it's best to wait until there are actual features you need.
> don't make changes for the sake of changes, there's always
> some risk.

Once we add the cache description (Ali's other set) can we make
sure we arbitrary decide to have separate cache structures.  The
earlier table version allowed sharing of the entrees in the table
which then became not allowed in the newer spec. That will smooth
the path quite a bit and is a valid way to interpret the earlier spec.

If we do that, we can bring the IDs + the version update as a
precursor to MPAM support series. I don't think we need them until
that series (which is a way off being ready to merge yet!)

Jonathan

> 
> > >> ---
> > >>  hw/acpi/aml-build.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > >> index 3010325ca423..e5401dfdb1a8 100644
> > >> --- a/hw/acpi/aml-build.c
> > >> +++ b/hw/acpi/aml-build.c
> > >> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> > >>      uint32_t pptt_start = table_data->len;
> > >>      uint32_t root_offset;
> > >>      int n;
> > >> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
> > >> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
> > >>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
> > >>  
> > >>      acpi_table_begin(&table, table_data);
> > >> -- 
> > >> 2.34.1  
> > > 
> > > 
> > > .
> > >   
> 
>
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Michael S. Tsirkin 6 months, 3 weeks ago
On Wed, Apr 23, 2025 at 04:47:20PM +0100, Jonathan Cameron wrote:
> On Wed, 23 Apr 2025 11:35:46 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Apr 23, 2025 at 10:15:42PM +0800, Yicong Yang wrote:
> > > On 2025/4/23 20:39, Michael S. Tsirkin wrote:  
> > > > On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee wrote:  
> > > >> From: Yicong Yang <yangyicong@hisilicon.com>
> > > >>
> > > >> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
> > > >> by handy. This is compatible with previous revision.
> > > >>
> > > >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > >> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>  
> > > > 
> > > > 
> > > > I don't get it. Why are you updating it? Which features
> > > > from the new one are you using?
> > > >   
> > > 
> > > no new features for this patchset. considered updating it to the latest ACPI
> > > spec since we're going to touch the PPTT table and tested data.  
> > 
> > it's best to wait until there are actual features you need.
> > don't make changes for the sake of changes, there's always
> > some risk.
> 
> Once we add the cache description (Ali's other set) can we make
> sure we arbitrary decide to have separate cache structures.  The
> earlier table version allowed sharing of the entrees in the table
> which then became not allowed in the newer spec. That will smooth
> the path quite a bit and is a valid way to interpret the earlier spec.
> 
> If we do that, we can bring the IDs + the version update as a
> precursor to MPAM support series. I don't think we need them until
> that series (which is a way off being ready to merge yet!)
> 
> Jonathan

Why not. Sounds like all that can be made part of Ali's patchset?
I am also ok to merge things gradually, as long as it's
clear and documented in the commit log why we are
making the change and where things are going.

> > 
> > > >> ---
> > > >>  hw/acpi/aml-build.c | 2 +-
> > > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > >> index 3010325ca423..e5401dfdb1a8 100644
> > > >> --- a/hw/acpi/aml-build.c
> > > >> +++ b/hw/acpi/aml-build.c
> > > >> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> > > >>      uint32_t pptt_start = table_data->len;
> > > >>      uint32_t root_offset;
> > > >>      int n;
> > > >> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
> > > >> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
> > > >>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
> > > >>  
> > > >>      acpi_table_begin(&table, table_data);
> > > >> -- 
> > > >> 2.34.1  
> > > > 
> > > > 
> > > > .
> > > >   
> > 
> >
Re: [PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
Posted by Alireza Sanaee via 6 months, 3 weeks ago
On Wed, 23 Apr 2025 11:51:41 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

Hi Michael,

> On Wed, Apr 23, 2025 at 04:47:20PM +0100, Jonathan Cameron wrote:
> > On Wed, 23 Apr 2025 11:35:46 -0400
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Wed, Apr 23, 2025 at 10:15:42PM +0800, Yicong Yang wrote:  
> > > > On 2025/4/23 20:39, Michael S. Tsirkin wrote:    
> > > > > On Wed, Apr 23, 2025 at 12:41:29PM +0100, Alireza Sanaee
> > > > > wrote:    
> > > > >> From: Yicong Yang <yangyicong@hisilicon.com>
> > > > >>
> > > > >> The lastest ACPI spec 6.5 support PPTT revision 3. Update it
> > > > >> by handy. This is compatible with previous revision.
> > > > >>
> > > > >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> > > > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > > > >> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>    
> > > > > 
> > > > > 
> > > > > I don't get it. Why are you updating it? Which features
> > > > > from the new one are you using?
> > > > >     
> > > > 
> > > > no new features for this patchset. considered updating it to
> > > > the latest ACPI spec since we're going to touch the PPTT table
> > > > and tested data.    
> > > 
> > > it's best to wait until there are actual features you need.
> > > don't make changes for the sake of changes, there's always
> > > some risk.  
> > 
> > Once we add the cache description (Ali's other set) can we make
> > sure we arbitrary decide to have separate cache structures.  The
> > earlier table version allowed sharing of the entrees in the table
> > which then became not allowed in the newer spec. That will smooth
> > the path quite a bit and is a valid way to interpret the earlier
> > spec.
> > 
> > If we do that, we can bring the IDs + the version update as a
> > precursor to MPAM support series. I don't think we need them until
> > that series (which is a way off being ready to merge yet!)
> > 
> > Jonathan  
> 
> Why not. Sounds like all that can be made part of Ali's patchset?
> I am also ok to merge things gradually, as long as it's
> clear and documented in the commit log why we are
> making the change and where things are going.
> 
> > >   
> > > > >> ---
> > > > >>  hw/acpi/aml-build.c | 2 +-
> > > > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >>
> > > > >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > >> index 3010325ca423..e5401dfdb1a8 100644
> > > > >> --- a/hw/acpi/aml-build.c
> > > > >> +++ b/hw/acpi/aml-build.c
> > > > >> @@ -2155,7 +2155,7 @@ void build_pptt(GArray *table_data,
> > > > >> BIOSLinker *linker, MachineState *ms, uint32_t pptt_start =
> > > > >> table_data->len; uint32_t root_offset;
> > > > >>      int n;
> > > > >> -    AcpiTable table = { .sig = "PPTT", .rev = 2,
> > > > >> +    AcpiTable table = { .sig = "PPTT", .rev = 3,
> > > > >>                          .oem_id = oem_id, .oem_table_id =
> > > > >> oem_table_id }; 
> > > > >>      acpi_table_begin(&table, table_data);
> > > > >> -- 
> > > > >> 2.34.1    
> > > > > 
> > > > > 
> > > > > .
> > > > >     
> > > 
> > >   
> 
> 
For this patchset, we eventually decided to rev down  for now, on
another version sent earlier. We can always rev up when it is
absolutely necessary. So it is now rev == 2.

Thanks,
Alireza