Re: [PATCH RFC 2/2] AMD IOMMU: allow command line overrides for broken IVRS tables

[Sander Eikelenboom <linux@eikelenboom.it>]

Tuesday, August 27, 2013, 10:05:41 AM, you wrote:

> With there being so many systems with broken ACPI tables, and with it
> generally being known what's wrong with those tables, give people a
> handle to overcome the resulting disabling of their IOMMUs.

> Inspired by Linux side patches providing similar functionality.

> TODO: documentation

Hi Jan,

Would the syntax be the same as linux ?
f.e. ivrs_ioapic[6]=00:14.0 for my case ?

I'm asking because using it on the command line seems to enable the iommu fine (which it shouldn't without the override), but i don't seem to see the
"IVHD: Command line override present for IO-APIC %#x" in my xl dmesg... 

--
Sander


> Suggested-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -81,9 +81,15 @@ void __init cmdline_parse(const char *cm
>          /* Search for value part of a key=value option. */
>          optval = strchr(opt, '=');
>          if ( optval != NULL )
> +        {
>              *optval++ = '\0'; /* nul-terminate the option value */
> +            q = strpbrk(opt, "([{<");
> +        }
>          else
> +        {
>              optval = q;       /* default option value is empty string */
> +            q = NULL;
> +        }
>  
>          /* Boolean parameters can be inverted with 'no-' prefix. */
>          bool_assert = !!strncmp("no-", optkey, 3);
> @@ -93,7 +99,17 @@ void __init cmdline_parse(const char *cm
>          for ( param = &__setup_start; param < &__setup_end; param++ )
>          {
>              if ( strcmp(param->name, optkey) )
> +            {
> +                if ( param->type == OPT_CUSTOM && q &&
> +                     strlen(param->name) == q + 1 - opt &&
> +                     !strncmp(param->name, opt, q + 1 - opt) )
> +                {
> +                    optval[-1] = '=';
> +                    ((void (*)(const char *))param->var)(q);
> +                    optval[-1] = '\0';
> +                }
>                  continue;
> +            }
>  
>              switch ( param->type )
>              {
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -633,6 +633,50 @@ static u16 __init parse_ivhd_device_exte
>      return dev_length;
>  }
>  
> +static __initdata DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
> +
> +static void __init parse_ivrs_ioapic(char *str)
> +{
> +    const char *s = str;
> +    unsigned long id;
> +    unsigned int seg, bus, dev, func;
> +
> +    ASSERT(*s == '[');
> +    id = simple_strtoul(s + 1, &s, 0);
+    if ( id >>= ARRAY_SIZE(ioapic_sbdf) || *s != ']' || *++s != '=' )
> +        return;
> +
> +    s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> +    if ( !s || *s )
> +        return;
> +
> +    ioapic_sbdf[id].bdf = PCI_BDF(bus, dev, func);
> +    ioapic_sbdf[id].seg = seg;
> +    __set_bit(id, ioapic_cmdline);
> +}
> +custom_param("ivrs_ioapic[", parse_ivrs_ioapic);
> +
> +static void __init parse_ivrs_hpet(char *str)
> +{
> +    const char *s = str;
> +    unsigned long id;
> +    unsigned int seg, bus, dev, func;
> +
> +    ASSERT(*s == '[');
> +    id = simple_strtoul(s + 1, &s, 0);
> +    if ( id != (typeof(hpet_sbdf.id))id || *s != ']' || *++s != '=' )
> +        return;
> +
> +    s = parse_pci(s + 1, &seg, &bus, &dev, &func);
> +    if ( !s || *s )
> +        return;
> +
> +    hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
> +    hpet_sbdf.seg = seg;
> +    hpet_sbdf.cmdline = 1;
> +}
> +custom_param("ivrs_hpet[", parse_ivrs_hpet);
> +
>  static u16 __init parse_ivhd_device_special(
>      const struct acpi_ivrs_device8c *special, u16 seg,
>      u16 header_length, u16 block_length, struct amd_iommu *iommu)
> @@ -674,7 +718,17 @@ static u16 __init parse_ivhd_device_spec
>              if ( IO_APIC_ID(apic) != special->handle )
>                  continue;
>  
> -            if ( ioapic_sbdf[special->handle].pin_2_idx )
> +            if ( special->handle >= ARRAY_SIZE(ioapic_sbdf) )
> +            {
> +                printk(XENLOG_ERR "IVHD Error: IO-APIC %#x entry beyond bounds\n",
> +                       special->handle);
> +                return 0;
> +            }
> +
> +            if ( test_bit(special->handle, ioapic_cmdline) )
> +                AMD_IOMMU_DEBUG("IVHD: Command line override present for IO-APIC %#x\n",
> +                                special->handle);
> +            else if ( ioapic_sbdf[special->handle].pin_2_idx )
>              {
>                  if ( ioapic_sbdf[special->handle].bdf == bdf &&
>                       ioapic_sbdf[special->handle].seg == seg )
> @@ -717,14 +771,18 @@ static u16 __init parse_ivhd_device_spec
>          break;
>      case ACPI_IVHD_HPET:
>          /* set device id of hpet */
> -        if ( hpet_sbdf.iommu )
> +        if ( hpet_sbdf.iommu ||
> +             (hpet_sbdf.cmdline && hpet_sbdf.id != special->handle) )
>          {
>              printk(XENLOG_WARNING "Only one IVHD HPET entry is supported\n");
>              break;
>          }
>          hpet_sbdf.id = special->handle;
> -        hpet_sbdf.bdf = bdf;
> -        hpet_sbdf.seg = seg;
> +        if ( !hpet_sbdf.cmdline )
> +        {
> +            hpet_sbdf.bdf = bdf;
> +            hpet_sbdf.seg = seg;
> +        }
>          hpet_sbdf.iommu = iommu;
>          break;
>      default:
> @@ -935,21 +993,23 @@ static int __init parse_ivrs_table(struc
>               ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
>              continue;
>  
> -        printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n",
> -               IO_APIC_ID(apic));
> -        if ( amd_iommu_perdev_intremap )
> -            error = -ENXIO;
> -        else
> +        if ( !test_bit(IO_APIC_ID(apic), ioapic_cmdline) )
>          {
> -            ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
> -                u16, nr_ioapic_entries[apic]);
> -            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
> -            {
> -                printk(XENLOG_ERR "IVHD Error: Out of memory\n");
> -                error = -ENOMEM;
> -            }
> +            printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n",
> +                   IO_APIC_ID(apic));
> +            if ( amd_iommu_perdev_intremap )
> +                return -ENXIO;
> +        }
> +
> +        ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx = xmalloc_array(
> +            u16, nr_ioapic_entries[apic]);
> +        if ( ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx )
>              memset(ioapic_sbdf[IO_APIC_ID(apic)].pin_2_idx, -1,
>                     nr_ioapic_entries[apic] * sizeof(*ioapic_sbdf->pin_2_idx));
> +        else
> +        {
> +            printk(XENLOG_ERR "IVHD Error: Out of memory\n");
> +            error = -ENOMEM;
>          }
>      }
>  
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
> @@ -108,6 +108,7 @@ extern struct ioapic_sbdf {
>  
>  extern struct hpet_sbdf {
>      u16 bdf, seg, id;
> +    bool_t cmdline;
>      struct amd_iommu *iommu;
>  } hpet_sbdf;
>