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

Re: [Xen-devel] [v5][PATCH 03/10] xen:x86: define a new hypercall to get RMRR mappings



On 2014/8/29 17:18, Jan Beulich wrote:
On 29.08.14 at 05:02, <tiejun.chen@xxxxxxxxx> wrote:
I tried to figure out solution as you suggestion but I'd like show my
draft design before post anything to review since please give some
suggestions here:

1. In the xen/include/xen/iommu.h file,

struct iommu_ops {
        ...
        int (*get_device_reserved_memory)(struct list_head 
*dev_reserved_memory);

2. In the xen/drivers/passthrough/vtd/iommu.c file,

extern int get_device_acpi_reserved_memory(struct list_head
*dev_reserved_memory);

const struct iommu_ops intel_iommu_ops = {
        ...
        .get_device_reserved_memory = get_device_acpi_reserved_memory,

3. In the xen/drivers/passthrough/vtd/dmar.c file,

struct list_head devices_reserved_memory = LIST_HEAD_INIT (
devices_reserved_memory );
int get_device_acpi_reserved_memory(struct list_head *dev_reserved_memory)
{
      static unsigned int device_reserved_memory_entries = 0;
      static unsigned int check_done = 0;
      struct acpi_rmrr_unit *rmrru;
      struct device_acpi_reserved_memory *darm = NULL;

      dev_reserved_memory = &devices_reserved_memory;

      if ( check_done )
          return device_reserved_memory_entries;
      else
      {
          list_for_each_entry(rmrru, &acpi_rmrr_units, list)
          {
              darm = xzalloc(struct device_acpi_reserved_memory);
              if ( !darm )
                  return -ENOMEM;

              darm->base_address = rmrru->base_address;
              darm->end_address = rmrru->end_address;
              list_add(&darm->list, &devices_reserved_memory);
              device_reserved_memory_entries++;
          }
      }

      check_done = 1;

      return device_reserved_memory_entries;
}

4. In the xen/include/asm-x86/acpi.h file,

+struct device_acpi_reserved_memory {
+    struct list_head list;
+    u64    base_address;
+    u64    end_address;
+};


Here a couple of questions:

1. Here I introduce this struct device_acpi_reserved_memory to avoid
exposing that existing structure and list acpi_rmrr_units

struct acpi_rmrr_unit {
      struct dmar_scope scope;
      struct list_head list;
      u64    base_address;
      u64    end_address;
      u16    segment;
      u8     allow_all:1;
};

Because:

1> Actually we just need two fields, base_address and end_address.
2> If reuse that structure, we still have to change some head files to
make sure we can use this in other files like I did in original patch #1
you don't like.

So what is your idea?

2. Based on your isolation policy, I don't expose acpi_rmrr_units
directly. Instead, I will copy this to another list,
devices_reserved_memory as I show above.

Is this reasonable and expected?

This still allocates another instance of structures to create a second
linked list. Did you consider get_device_reserved_memory() to take

Do you mean we still use this existing type combo, acpi_rmrr_units and acpi_rmrr_units?

a callback function instead?


But we should do something like this,

1. .get_device_reserved_memory = get_drm_all,
2.  static int get_drm_all(struct list_head *dev_reserved_memory)
    {
        return (get_drm_callback(dev_reserved_memory));
    }

3. get_drm_callback = get_device_acpi_reserved_memory;
4. static int get_device_acpi_reserved_memory(struct list_head *dev_reserved_memory)
    {
        ...
        dev_reserved_memory = &acpi_rmrr_units;
        ...
    }

Then while calling the hypercall,

struct list_head *dev_reserved_memory = NULL;
nr_entries = ops->get_device_reserved_memory(dev_reserved_memory);
if (!nr_entries)
        list_for_each_entry( darm, dev_reserved_memory, list )
        {
                xxx.start_pfn = ...;
                xxx.nr_pages = ...;
                if ( copy_to_guest_offset(buffer, i, &xxx, 1) )
                ...
        }

Thanks
Tiejun

_______________________________________________
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®.