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

Re: [Xen-devel] [v8][PATCH 16/17] xen/vtd: group assigned device with RMRR



>>> On 01.12.14 at 10:24, <tiejun.chen@xxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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