>>> On 01.12.14 at 10:24, <tiejun.c...@intel.com> wrote:
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -572,10 +572,11 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
> {
> struct acpi_dmar_reserved_memory *rmrr =
> container_of(header, struct acpi_dmar_reserved_memory, header);
> - struct acpi_rmrr_unit *rmrru;
> + struct acpi_rmrr_unit *rmrru, *cur_rmrr;
> void *dev_scope_start, *dev_scope_end;
> u64 base_addr = rmrr->base_address, end_addr = rmrr->end_address;
> int ret;
> + static unsigned int group_id = 0;
__initdata. Pointless initializer.
> @@ -682,7 +685,30 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header)
> "So please set pci_rdmforce to reserve these ranges"
> " if you need such a device in hotplug case.\n");
>
> + list_for_each_entry(cur_rmrr, &acpi_rmrr_units, list)
> + {
> + /*
> + * Any same or overlap range mean they should be
> + * at same group.
> + */
> + if ( ((base_addr >= cur_rmrr->base_address) &&
> + (end_addr <= cur_rmrr->end_address)) ||
> + ((base_addr <= cur_rmrr->base_address) &&
> + (end_addr >= cur_rmrr->end_address)) )
This is both more complicated than needed and wrong. You want
an overlap (partial or complete doesn't matter) check, i.e.
start1 <= end2 && start2 <= end1.
> + {
> + rmrru->gid = cur_rmrr->gid;
> + continue;
break
Also this doesn't seem to handle cases where you see in this order
[2,3]
[4,6]
[3,5]
But the more fundamental question is: Are overlaps of RMRRs
actually allowed, or would it not better to bail in that case and
leave the IOMMU disabled?
But the code further down looks so broken that I'll leave to you
to discuss this with your colleagues acting as VT-d maintainers.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
