[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 2/8] AMD/IOMMU: obtain IVHD type to use earlier


  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Thu, 26 Aug 2021 13:30:29 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=fnqIGO1od2TJLf2Vm2JprfDfPfoJAOarWZHkjJuVeE8=; b=HCr/bKAjo+vbJw4oKQT5FthiG08cCp9Ayiuo7ry9EppzefSo5KJpk6V5mmLav9mULqs41tXeqtWAG/rjg8C4C/G5EL48hhkBofoq80X/4RbBq+fHMqcnUwfDFLMIeCogVK80XBYSdU/oVRLtr1jqqp7O/CJwo+Q+FjhSoN0cKktzktu8y9uBkfQkqYA3jDbqjvEjNPWL4nOfqsq7D94011Ngde8q6dwMKpIvS3PXNT+cgnJW5WR/J2AJH7lmycmQVyn08TMgIQe0otdpuwWSMg6TaTaVyuoPhfn/cgGuw7QiIBjQtbtEWdGuhZUWFc5NN3yaPG8PrRPiqE+OnAhk+w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OgKjnqwl9WrZbIy/Icx7Mis5laP/2yx/bxhUmzCwkYcbQBEEFITkDpHOJ4s2Uw/ASNfZdiglAAlDtKM1QbJupKhqmmW7eqLPN0yw4Ae2uItC1qTqHFbIVeKTD6g3QoLDvB+yUJg960YtAAGEdLgQCEha/eQfxAV05zfWnpqHjFH3VAGXTOI7/jusnU9yxXMLiLjgeAuG/s3/7iITqh/3ra2IgnD3Bsm/X9aa2UTwjoAQo+V3WlgjPIZEdOH0+9AGdZZk+U+sxLw3tmfFAk+syqt52y8IngBM63U/XcwmfQorHRqEqsA2/tWroV7c/yVZzKHRLpBEyUc2HWpCJpX66Q==
  • Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Thu, 26 Aug 2021 12:30:55 +0000
  • Ironport-hdrordr: A9a23:WdkN9a5iBytRcS0syAPXwaaEI+orL9Y04lQ7vn2ZFiY6TiXIra +TdaoguSMc0AxhJU3I6urwRJVoIEmsuqKdhLN7XItKMzOWwVdAQLsSj7cKhgeQYREWldQtm5 uIEZIOcOEYZGIS5aubkWqF+pQbsaK6GcuT9IPjJgJWPGdXgtZbnmBE42igYyhLrQ99b6YRJd 653I5qtjCgcXMYYoCQHX8eRdHZq9nKjp79JTYbGh8O8mC1/GiVwY+/NyLd8gYVUjtJz7tn23 PCiRbB/amms+u20Fv1y3LTy5hdmdPnysFGbfb80PQ9G3HJsEKFdY5hU7qNsHQeu+e08m8wnN 3NuRs7e+xu9nLqeH2vqxeF4Xi87B8er1vZjXOIi3rqpsL0ABggDdBavJlUdhvC51Bll/tXuZ g7jl6xht5yN1ftjS7979/HW1VBjUyvu0M4neoSlXBEFbE1TdZq3NMi1XIQNK1FMDPx6YghHu UrJtrb/uxual+fb2rUpC1T29SqRG00BQq6WUAMtcye7ihOkBlCvhcl7f1auk1F2IM2SpFC6e iBGL9vjqt2VcMTbbhwHqMIRta2EHXERVbXOmqUK1LhCa0XJhv22szKyYRwwNvvVI0DzZM0lp iEekhfr3QKYE7rCdeDxtlJ9AzXR225UTmo18cb/JRyuqT9SL33K0S4OQUTuvrlh89aLtzQWv 61Np4TKeTkN3HWAopM3xfzQd1XJWMFWMMYoNAmQFiDy/i7dLHCh6j+SrL+NbDtGTErVifUGX 0YRgHpKMFB9EywHnnigBbQXHvpcEn+8ZVsELHT8uUJxJIWX7c84jQ9uBCc3IWmODdCuqs5cA 9VO7X8iJqhqW2352rTq3xzPBBQFFxY7fHqQzdIoxMQNEvwcbEM0u/vLVx67T+iHFtSXsnWGA lQqxBc4qSsNaWKySQjEd68dn+Ah3wIvXSQUooGkqKN5c35aoo1Z6xWC5BZJEHuLVhYiAxqoG BMZEsvXUnEDA7jjq2jkdg9GPzfX8MUunbqHedk7Vbk8WmMr8AmQXUWGxS0V9SMvAooTz1Iwn Vs7q4khqaakzrHExp9vA14CiwPVI2kOsMDMO23XvQSplktQnA3cY6+v03et/j0QBuyy6xdvB 24EcTeQ4C1PrMUgAEn7k9ByiIiSozaRTMwVpkyi/wNKU3W/nl0yuOFfay1zi+YbUYD2PgUNH XfbSIVOR4G/aH96Pe5okfKKZwd/ORnAgUdNsVXT5jDnne2bIGYn6APGPFZuJ5jKdD1q+cOFe aSYRWcIj/0A/4gn1X9nAdpBABk7H0/1f/40hzs62a1mHY5HPrJOVxjA7UWOcuV4WToT+uBlJ 95kdU2t+2tNXiZUK/N9YjHKzpYbh/Dq2+/SO8l7ZhSoKIprbN2W4LWVDPZvUs3qinW7P2E43 /2ZZ4LrIwpFrUfDPD6ShgpimbBzu7/XHfDmjaGcNMDQQ==
  • Ironport-sdr: ISB3iveNyVExZSYpqgzQsFparhe/OGme9PNPfyX90zphmMkpk22moBg+2UIPap5uO3OKzY9pB4 tGoiFx+YmgMNyNpd87IMVnwnB5Tuz7LUg1v+dM1XqDDMvPfjgh7DskcacMxGnmnER//tJrtZC7 /XHzVLfLV61cLa2aBRkzRwbYp18eGy+6JQXzv35b6RVCPYkcs0mPk+poBeI81Fm6i08L5XU/V6 iOA655+LGR9CWi2mCRA11/wqUUKwe9jt04dy99tAycWcMjmoHCnPpVTJy6S8rbcJuCU8SBuXOQ KL2i8lLC60+4OMQEh/3KHheD
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.