On 26/08/2021 08:23, Jan Beulich wrote:
> Doing this in amd_iommu_prepare() is too late for it, in particular, to
> be used in amd_iommu_detect_one_acpi(), as a subsequent change will want
> to do. Moving it immediately ahead of amd_iommu_detect_acpi() is
> (luckily) pretty simple, (pretty importantly) without breaking
> amd_iommu_prepare()'s logic to prevent multiple processing.
>
> This involves moving table checksumming, as
> amd_iommu_get_supported_ivhd_type() -> get_supported_ivhd_type() will
> now be invoked before amd_iommu_detect_acpi() -> detect_iommu_acpi(). In
> the course of dojng so stop open-coding acpi_tb_checksum(), seeing that
doing.
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -1150,20 +1152,7 @@ static int __init parse_ivrs_table(struc
> static int __init detect_iommu_acpi(struct acpi_table_header *table)
> {
> const struct acpi_ivrs_header *ivrs_block;
> - unsigned long i;
> unsigned long length = sizeof(struct acpi_table_ivrs);
> - u8 checksum, *raw_table;
> -
> - /* validate checksum: sum of entire table == 0 */
> - checksum = 0;
> - raw_table = (u8 *)table;
> - for ( i = 0; i < table->length; i++ )
> - checksum += raw_table[i];
> - if ( checksum )
> - {
> - AMD_IOMMU_DEBUG("IVRS Error: Invalid Checksum %#x\n", checksum);
> - return -ENODEV;
> - }
>
> while ( table->length > (length + sizeof(*ivrs_block)) )
> {
> @@ -1300,6 +1289,15 @@ get_supported_ivhd_type(struct acpi_tabl
> {
> size_t length = sizeof(struct acpi_table_ivrs);
> const struct acpi_ivrs_header *ivrs_block, *blk = NULL;
> + uint8_t checksum;
> +
> + /* Validate checksum: Sum of entire table == 0. */
> + checksum = acpi_tb_checksum(ACPI_CAST_PTR(uint8_t, table),
> table->length);
> + if ( checksum )
> + {
> + AMD_IOMMU_DEBUG("IVRS Error: Invalid Checksum %#x\n", checksum);
I know you're just moving code, but this really needs to be a visible
error. It's "I'm turning off the IOMMU because the ACPI table is bad",
which is about as serious as errors come.
~Andrew
