|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8 4/4] iommu: add rmrr Xen command line option for extra rmrrs
. snip..
> diff --git a/xen/drivers/passthrough/vtd/dmar.c
> b/xen/drivers/passthrough/vtd/dmar.c
> index a8e1e5d..fa659a9 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -42,6 +42,8 @@
> #define MIN_SCOPE_LEN (sizeof(struct acpi_dmar_device_scope) + \
> sizeof(struct acpi_dmar_pci_path))
>
> +#define PRI_RMRR(s,e) "[%lx-%lx]"
> +
> LIST_HEAD_READ_MOSTLY(acpi_drhd_units);
> LIST_HEAD_READ_MOSTLY(acpi_rmrr_units);
> static LIST_HEAD_READ_MOSTLY(acpi_atsr_units);
> @@ -425,7 +427,7 @@ static int __init acpi_parse_dev_scope(
> default:
> if ( iommu_verbose )
> printk(XENLOG_WARNING VTDPREFIX "Unknown scope type %#x\n",
> - acpi_scope->entry_type);
> + acpi_scope->entry_type);
Stray change? It should be in a seperate patch I think?
> start += acpi_scope->length;
> continue;
> }
> @@ -479,8 +481,7 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
> INIT_LIST_HEAD(&dmaru->ioapic_list);
> INIT_LIST_HEAD(&dmaru->hpet_list);
> if ( iommu_verbose )
> - dprintk(VTDPREFIX, " dmaru->address = %"PRIx64"\n",
> - dmaru->address);
> + dprintk(VTDPREFIX, " dmaru->address = %"PRIx64"\n", dmaru->address);
Ditto.
>
> ret = iommu_alloc(dmaru);
> if ( ret )
> @@ -541,8 +542,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
> if ( !pci_device_detect(drhd->segment, b, d, f) )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " Non-existent device (%04x:%02x:%02x.%u) is
> reported"
> - " in this DRHD's scope!\n", drhd->segment, b, d, f);
> + " Non-existent device (%04x:%02x:%02x.%u) is
> reported in this DRHD's scope!\n",
> + drhd->segment, b, d, f);
As well - an seperate cleanup patch.
> invalid_cnt++;
> }
> }
> @@ -553,8 +554,8 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
> invalid_cnt == dmaru->scope.devices_cnt )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " Workaround BIOS bug: ignore the DRHD due to all "
> - "devices under its scope are not PCI discoverable!\n");
> + " Workaround BIOS bug: ignore the DRHD due to all "
> + "devices under its scope are not PCI
> discoverable!\n");
This one too.
>
> scope_devices_free(&dmaru->scope);
> iommu_free(dmaru);
> @@ -563,10 +564,10 @@ acpi_parse_one_drhd(struct acpi_dmar_header *header)
> else
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " The DRHD is invalid due to there are devices under "
> - "its scope are not PCI discoverable! Pls try option "
> - "iommu=force or iommu=workaround_bios_bug if you "
> - "really want VT-d\n");
> + " The DRHD is invalid due to there are devices
> under "
> + "its scope are not PCI discoverable! Pls try option "
> + "iommu=force or iommu=workaround_bios_bug if you "
> + "really want VT-d\n");
And that as well.
> ret = -EINVAL;
> }
> }
> @@ -604,8 +605,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " Non-existent device (%04x:%02x:%02x.%u) is reported"
> - " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> + " Non-existent device (%04x:%02x:%02x.%u) is reported in
> RMRR "PRI_RMRR(s,e)"'s scope!\n",
> rmrru->segment, b, d, f,
> rmrru->base_address, rmrru->end_address);
> ignore = 1;
> @@ -620,16 +620,16 @@ static int register_one_rmrr(struct acpi_rmrr_unit
> *rmrru)
> if ( ignore )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> + " Ignore the RMRR "PRI_RMRR(s,e)" due to "
> "devices under its scope are not PCI discoverable!\n",
> - rmrru->base_address, rmrru->end_address);
> + rmrru->base_address, rmrru->end_address);
> scope_devices_free(&rmrru->scope);
> xfree(rmrru);
> }
> else if ( rmrru->base_address > rmrru->end_address )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
> + " The RMRR "PRI_RMRR(s,e)" is incorrect!\n",
> rmrru->base_address, rmrru->end_address);
> scope_devices_free(&rmrru->scope);
> xfree(rmrru);
> @@ -639,7 +639,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> {
> if ( iommu_verbose )
> dprintk(VTDPREFIX,
> - " RMRR region: base_addr %"PRIx64" end_address
> %"PRIx64"\n",
> + " RMRR region: "PRI_RMRR(s,e)"\n",
> rmrru->base_address, rmrru->end_address);
> acpi_register_rmrr_unit(rmrru);
> }
> @@ -664,7 +664,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
> if ( base_addr <= rmrru->end_address && rmrru->base_address <=
> end_addr )
> {
> printk(XENLOG_ERR VTDPREFIX
> - "Overlapping RMRRs [%"PRIx64",%"PRIx64"] and
> [%"PRIx64",%"PRIx64"]\n",
> + "Overlapping RMRRs "PRI_RMRR(s,e)" and "PRI_RMRR(s,e)"\n",
> rmrru->base_address, rmrru->end_address,
> base_addr, end_addr);
> return -EEXIST;
> @@ -678,8 +678,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
> (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) )
> {
> dprintk(XENLOG_WARNING VTDPREFIX,
> - " RMRR address range not in reserved memory "
> - "base = %"PRIx64" end = %"PRIx64"; "
> + " RMRR address range "PRI_RMRR(s,e)" not in reserved
> memory; "
> "iommu_inclusive_mapping=1 parameter may be needed.\n",
> base_addr, end_addr);
> }
> @@ -779,8 +778,7 @@ acpi_parse_one_rhsa(struct acpi_dmar_header *header)
> list_add_tail(&rhsau->list, &acpi_rhsa_units);
> if ( iommu_verbose )
> dprintk(VTDPREFIX,
> - " rhsau->address: %"PRIx64
> - " rhsau->proximity_domain: %"PRIx32"\n",
> + " rhsau->address: %"PRIx64" rhsau->proximity_domain:
> %"PRIx32"\n",
All of those should go in a seperate cleanup patch please.
> rhsau->address, rhsau->proximity_domain);
>
> return ret;
> @@ -869,6 +867,140 @@ out:
> return ret;
> }
>
> +#define MAX_EXTRA_RMRR_PAGES 16
> +#define MAX_EXTRA_RMRR 10
> +
> +/* RMRR units derived from command line rmrr option */
Missing full stop.
> +#define MAX_EXTRA_RMRR_DEV 20
> +struct extra_rmrr_unit {
> + struct list_head list;
> + unsigned long base_pfn, end_pfn;
> + unsigned int dev_count;
> + u32 sbdf[MAX_EXTRA_RMRR_DEV];
> +};
> +static __initdata unsigned int nr_rmrr;
> +static struct __initdata extra_rmrr_unit extra_rmrr_units[MAX_EXTRA_RMRR];
> +
> +/* Macro for RMRR inclusive range formatting */
Macro? Ah I think you meant the:
#define PRI_RMRR(s,e) "[%lx-%lx]"
which is defined somewhere at the top of the file with your patch. Please
move it here.
Also, missing full stop at the end of the line.
> +
> +static void __init add_extra_rmrr(void)
> +{
> + struct acpi_rmrr_unit *acpi_rmrr;
> + struct acpi_rmrr_unit *rmrru;
> + unsigned int dev, seg, i, j;
> + unsigned long pfn;
> +
> + for ( i = 0; i < nr_rmrr; i++ )
> + {
> + if ( extra_rmrr_units[i].base_pfn > extra_rmrr_units[i].end_pfn )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Invalid RMRR Range "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn);
> + continue;
> + }
> +
> + if ( extra_rmrr_units[i].end_pfn - extra_rmrr_units[i].base_pfn >=
> + MAX_EXTRA_RMRR_PAGES )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "RMRR range "PRI_RMRR(s,e)" exceeds
> "__stringify(MAX_EXTRA_RMRR_PAGES)" pages\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn);
> + continue;
> + }
> +
> + for ( j = 0; j < nr_rmrr; j++ )
> + {
> + if ( i != j &&
> + extra_rmrr_units[i].base_pfn <= extra_rmrr_units[j].end_pfn
> &&
> + extra_rmrr_units[j].base_pfn <= extra_rmrr_units[i].end_pfn
> )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Overlapping RMRRs "PRI_RMRR(s,e)" and
> "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn,
> + extra_rmrr_units[j].base_pfn,
> extra_rmrr_units[j].end_pfn);
> + break;
> + }
> + }
> + /* Broke out of the overlap loop check, continue with next rmrr. */
> + if ( j < nr_rmrr )
> + continue;
> + list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> + {
> + if ( pfn_to_paddr(extra_rmrr_units[i].base_pfn <=
> rmrru->end_address) &&
> + rmrru->base_address <=
> pfn_to_paddr(extra_rmrr_units[i].end_pfn) )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Overlapping extra RMRRs "PRI_RMRR(s,e)" and ACPI
> RMRRs "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> + extra_rmrr_units[i].end_pfn,
> + paddr_to_pfn(rmrru->base_address),
> + paddr_to_pfn(rmrru->end_address));
> + break;
You break out of the for loop (list_for_each_entry) - did ..
> + }
> + }
... you also want to continue with the next rmrr? Right now the code
will continue on with this 'i'.
> +
> +
> + pfn = extra_rmrr_units[i].base_pfn;
> + do
> + {
> + if ( !mfn_valid(pfn) || (pfn >> (paddr_bits - PAGE_SHIFT)) )
> + {
> + if ( iommu_verbose )
Why the 'iommu_verbose' ? Why not just print it out as all the other errors
are printed?
> + printk(XENLOG_ERR VTDPREFIX
> + "Invalid mfn in RMRR range "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn);
> + break;
> + }
> + } while ( pfn++ <= extra_rmrr_units[i].end_pfn );
> + /* The range had invalid mfn as the loop was broken out before
> reaching end_pfn. */
> + if ( pfn <= extra_rmrr_units[i].end_pfn )
> + continue;
> +
> + acpi_rmrr = xzalloc(struct acpi_rmrr_unit);
> + if ( !acpi_rmrr )
> + return;
> +
> + acpi_rmrr->scope.devices = xmalloc_array(u16,
> +
> extra_rmrr_units[i].dev_count);
> + if ( !acpi_rmrr->scope.devices )
> + {
> + xfree(acpi_rmrr);
> + return;
> + }
> +
> + seg = 0;
> + for ( dev = 0; dev < extra_rmrr_units[i].dev_count; dev++ )
> + {
> + acpi_rmrr->scope.devices[dev] = extra_rmrr_units[i].sbdf[dev];
> + seg = seg | PCI_SEG(extra_rmrr_units[i].sbdf[dev]);
> + }
> + if ( seg != PCI_SEG(extra_rmrr_units[i].sbdf[0]) )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Segments are not equal for RMRR range "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn);
> + scope_devices_free(&acpi_rmrr->scope);
> + xfree(acpi_rmrr);
> + continue;
> + }
> +
> + acpi_rmrr->segment = seg;
> + acpi_rmrr->base_address = pfn_to_paddr(extra_rmrr_units[i].base_pfn);
> + acpi_rmrr->end_address = pfn_to_paddr(extra_rmrr_units[i].end_pfn +
> 1);
> + acpi_rmrr->scope.devices_cnt = extra_rmrr_units[i].dev_count;
> +
> + if ( register_one_rmrr(acpi_rmrr) )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Could not register RMMR range "PRI_RMRR(s,e)"\n",
> + extra_rmrr_units[i].base_pfn,
> extra_rmrr_units[i].end_pfn);
> + scope_devices_free(&acpi_rmrr->scope);
> + xfree(acpi_rmrr);
> + }
> + }
> +}
> +
> #include <asm/tboot.h>
> /* ACPI tables may not be DMA protected by tboot, so use DMAR copy */
> /* SINIT saved in SinitMleData in TXT heap (which is DMA protected) */
> @@ -878,6 +1010,7 @@ int __init acpi_dmar_init(void)
> {
> acpi_physical_address dmar_addr;
> acpi_native_uint dmar_len;
> + int ret;
>
> if ( ACPI_SUCCESS(acpi_get_table_phys(ACPI_SIG_DMAR, 0,
> &dmar_addr, &dmar_len)) )
> @@ -888,7 +1021,10 @@ int __init acpi_dmar_init(void)
> dmar_table = __va(dmar_addr);
> }
>
> - return parse_dmar_table(acpi_parse_dmar);
> + ret = parse_dmar_table(acpi_parse_dmar);
> + add_extra_rmrr();
> +
> + return ret;
> }
>
> void acpi_dmar_reinstate(void)
> @@ -919,3 +1055,67 @@ int platform_supports_x2apic(void)
> unsigned int mask = ACPI_DMAR_INTR_REMAP | ACPI_DMAR_X2APIC_OPT_OUT;
> return cpu_has_x2apic && ((dmar_flags & mask) == ACPI_DMAR_INTR_REMAP);
> }
> +
> +/*
> + * Parse rmrr Xen command line options and add parsed device and region into
> + * acpi_rmrr_unit list to mapped as RMRRs parsed from ACPI.
> + * Format:
> + *
> rmrr=start<-end>=[s1]bdf1[,[s1]bdf2[,...]];start<-end>=[s2]bdf1[,[s2]bdf2[,...]]
> + * If the segment of the first device is not specified, segment zero will be
> used.
> + * If other segments are not specified, first device segment will be used.
> + * If a segment is specified for other than the first device and it does not
> match
> + * the one specified for the first one, an error will be reported.
> + */
> +static void __init parse_rmrr_param(const char *str)
> +{
> + const char *s = str, *cur, *stmp;
> + unsigned int seg, bus, dev, func;
> + unsigned long start, end;
> +
> + do {
> + start = simple_strtoul(cur = s, &s, 0);
> + if ( cur == s )
> + break;
> +
> + if ( *s == '-' )
> + {
> + end = simple_strtoul(cur = s + 1, &s, 0);
> + if ( cur == s )
> + break;
> + }
> + else
> + end = start;
> +
> + extra_rmrr_units[nr_rmrr].base_pfn = start;
> + extra_rmrr_units[nr_rmrr].end_pfn = end;
> + extra_rmrr_units[nr_rmrr].dev_count = 0;
> +
> + if ( *s != '=' )
> + continue;
> +
> + do {
> + bool_t default_segment = 0;
> +
> + if ( *s == ';' )
> + break;
> + stmp = __parse_pci(s + 1, &seg, &bus, &dev, &func,
> &default_segment);
> + if ( !stmp )
> + break;
> +
> + /* Not specified segment will be replaced with one from first
> device. */
> + if ( extra_rmrr_units[nr_rmrr].dev_count && default_segment )
> + seg = PCI_SEG(extra_rmrr_units[nr_rmrr].sbdf[0]);
> +
> + /* Keep sbdf's even if they differ and later report an error. */
> +
> extra_rmrr_units[nr_rmrr].sbdf[extra_rmrr_units[nr_rmrr].dev_count] =
> PCI_SBDF(seg, bus, dev, func);
> + extra_rmrr_units[nr_rmrr].dev_count++;
> + s = stmp;
> + } while ( *s == ',' &&
> + extra_rmrr_units[nr_rmrr].dev_count < MAX_EXTRA_RMRR_DEV );
> +
> + if ( extra_rmrr_units[nr_rmrr].dev_count )
> + nr_rmrr++;
> +
> + } while ( *s++ == ';' && nr_rmrr < MAX_EXTRA_RMRR );
> +}
> +custom_param("rmrr", parse_rmrr_param);
Otherwise looks fine!
> --
> 2.1.3
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |