Question about FADT length

Question about FADT length

[Hanjun Guo]

Hi,

I ran into some compile errors when I was prototyping the code
for the proposals in ASWG, it turns out that some field in FADT is
changed and "reserved4[0]" used in ACPI_FADT_V2_SIZE will
cause the error.

So my question is that the length of very version of FADT is
fixed, why not use a constant value for this puspose?

Here is the code:

/*
  * Sizes of the various flavors of FADT. We need to look closely
  * at the FADT length because the version number essentially tells
  * us nothing because of many BIOS bugs where the version does not
  * match the expected length. In other words, the length of the
  * FADT is the bottom line as to what the version really is.
  *
  * For reference, the values below are as follows:
  *     FADT V1  size: 0x074
  *     FADT V2  size: 0x084
  *     FADT V3  size: 0x0F4
  *     FADT V4  size: 0x0F4
  *     FADT V5  size: 0x10C
  */
#define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
#define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
#define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
#define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))

Why not  #define ACPI_FADT_V1_SIZE 0x074 ?
Did I miss something? any clarify will be appreciated :)

Thanks
Hanjun

RE: Question about FADT length

[Zheng, Lv]

Hi,

Using size instead of version field to determine FADT version has been talked for many times.
I didn't see a real case that was persuasive enough to introduce this change.
Do you have real platforms for us to examine?

Thanks and best regards
-Lv

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Hanjun Guo
> Sent: Wednesday, May 07, 2014 4:14 PM
> 
> Hi,
> 
> I ran into some compile errors when I was prototyping the code
> for the proposals in ASWG, it turns out that some field in FADT is
> changed and "reserved4[0]" used in ACPI_FADT_V2_SIZE will
> cause the error.
> 
> So my question is that the length of very version of FADT is
> fixed, why not use a constant value for this puspose?
> 
> Here is the code:
> 
> /*
>   * Sizes of the various flavors of FADT. We need to look closely
>   * at the FADT length because the version number essentially tells
>   * us nothing because of many BIOS bugs where the version does not
>   * match the expected length. In other words, the length of the
>   * FADT is the bottom line as to what the version really is.
>   *
>   * For reference, the values below are as follows:
>   *     FADT V1  size: 0x074
>   *     FADT V2  size: 0x084
>   *     FADT V3  size: 0x0F4
>   *     FADT V4  size: 0x0F4
>   *     FADT V5  size: 0x10C
>   */
> #define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
> #define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
> #define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
> #define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))
> 
> Why not  #define ACPI_FADT_V1_SIZE 0x074 ?
> Did I miss something? any clarify will be appreciated :)
> 
> Thanks
> Hanjun
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about FADT length

[Hanjun Guo]

Hi Lv,

On 2014-5-9 9:45, Zheng, Lv wrote:
> Hi,
> 
> Using size instead of version field to determine FADT version has been talked for many times.
> I didn't see a real case that was persuasive enough to introduce this change.
> Do you have real platforms for us to examine?

There are some changes for the FADT in the new coming ACPI spec
which changed the field of "reserved4" into other useful information,
and the name of that field was changed also. So if we want to accommodate
that change, we should modify the "reserved4" in the macro of
ACPI_FADT_V2_SIZE too. If any change to the FADT in the future, we may
modify it again, so I think constant value for the macros would
be better.

Thanks
Hanjun

> 
> Thanks and best regards
> -Lv
> 
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Hanjun Guo
>> Sent: Wednesday, May 07, 2014 4:14 PM
>>
>> Hi,
>>
>> I ran into some compile errors when I was prototyping the code
>> for the proposals in ASWG, it turns out that some field in FADT is
>> changed and "reserved4[0]" used in ACPI_FADT_V2_SIZE will
>> cause the error.
>>
>> So my question is that the length of very version of FADT is
>> fixed, why not use a constant value for this puspose?
>>
>> Here is the code:
>>
>> /*
>>   * Sizes of the various flavors of FADT. We need to look closely
>>   * at the FADT length because the version number essentially tells
>>   * us nothing because of many BIOS bugs where the version does not
>>   * match the expected length. In other words, the length of the
>>   * FADT is the bottom line as to what the version really is.
>>   *
>>   * For reference, the values below are as follows:
>>   *     FADT V1  size: 0x074
>>   *     FADT V2  size: 0x084
>>   *     FADT V3  size: 0x0F4
>>   *     FADT V4  size: 0x0F4
>>   *     FADT V5  size: 0x10C
>>   */
>> #define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
>> #define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
>> #define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
>> #define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))
>>
>> Why not  #define ACPI_FADT_V1_SIZE 0x074 ?
>> Did I miss something? any clarify will be appreciated :)
>>
>> Thanks
>> Hanjun
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Question about FADT length

[Zheng, Lv]

Hi,

> From: Hanjun Guo [mailto:hanjun.guo@linaro.org]
> Sent: Friday, May 09, 2014 4:45 PM
> 
> Hi Lv,
> 
> On 2014-5-9 9:45, Zheng, Lv wrote:
> > Hi,
> >
> > Using size instead of version field to determine FADT version has been talked for many times.
> > I didn't see a real case that was persuasive enough to introduce this change.
> > Do you have real platforms for us to examine?
> 
> There are some changes for the FADT in the new coming ACPI spec
> which changed the field of "reserved4" into other useful information,
> and the name of that field was changed also. So if we want to accommodate
> that change, we should modify the "reserved4" in the macro of
> ACPI_FADT_V2_SIZE too.

Is it possible you just write a patch to change the reserved4 and the macro using the new field name after the release of the spec?

If not, you can write a patch to change the macros, but don't forget to update the comments for the macros.
You could use the field names to replace the constant values in the comments. 

> If any change to the FADT in the future, we may
> modify it again, so I think constant value for the macros would
> be better.

There might not be such change after the release of the spec.

Thanks
-Lv

> 
> Thanks
> Hanjun
> 
> >
> > Thanks and best regards
> > -Lv
> >
> >> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Hanjun Guo
> >> Sent: Wednesday, May 07, 2014 4:14 PM
> >>
> >> Hi,
> >>
> >> I ran into some compile errors when I was prototyping the code
> >> for the proposals in ASWG, it turns out that some field in FADT is
> >> changed and "reserved4[0]" used in ACPI_FADT_V2_SIZE will
> >> cause the error.
> >>
> >> So my question is that the length of very version of FADT is
> >> fixed, why not use a constant value for this puspose?
> >>
> >> Here is the code:
> >>
> >> /*
> >>   * Sizes of the various flavors of FADT. We need to look closely
> >>   * at the FADT length because the version number essentially tells
> >>   * us nothing because of many BIOS bugs where the version does not
> >>   * match the expected length. In other words, the length of the
> >>   * FADT is the bottom line as to what the version really is.
> >>   *
> >>   * For reference, the values below are as follows:
> >>   *     FADT V1  size: 0x074
> >>   *     FADT V2  size: 0x084
> >>   *     FADT V3  size: 0x0F4
> >>   *     FADT V4  size: 0x0F4
> >>   *     FADT V5  size: 0x10C
> >>   */
> >> #define ACPI_FADT_V1_SIZE       (u32) (ACPI_FADT_OFFSET (flags) + 4)
> >> #define ACPI_FADT_V2_SIZE       (u32) (ACPI_FADT_OFFSET (reserved4[0]) + 3)
> >> #define ACPI_FADT_V3_SIZE       (u32) (ACPI_FADT_OFFSET (sleep_control))
> >> #define ACPI_FADT_V5_SIZE       (u32) (sizeof (struct acpi_table_fadt))
> >>
> >> Why not  #define ACPI_FADT_V1_SIZE 0x074 ?
> >> Did I miss something? any clarify will be appreciated :)
> >>
> >> Thanks
> >> Hanjun
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html