Re: [v7][RFC][PATCH 06/13] hvmloader/ram: check if guest memory is out of reserved device memory maps

["Jan Beulich" <JBeulich@suse.com>]

>>> On 06.11.14 at 10:28, <tiejun.chen@intel.com> wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -484,6 +484,14 @@ struct xen_domctl_assign_device {
>   typedef struct xen_domctl_assign_device xen_domctl_assign_device_t;
>   DEFINE_XEN_GUEST_HANDLE(xen_domctl_assign_device_t);
> 
> +/* Control whether/how we check and reserve device memory. */
> +struct xen_domctl_set_rdm {
> +    uint32_t  pci_rdmforce;
> +    uint32_t  pci_dev_bitmap;

So this allows for 32 devices; considering that you index the bitmap
by BDF, all this covers are 0000:00:00.0 through 0000:00:03.7.
Hardly enough to cover even a single pass through device (iirc the
range above is fully used by emulated devices). And of course a
much larger bitmap isn't a good solution either.

Nor am I really clear what you need the 32 bits of "pci_rdmforce"
for, nor why this field isn't just being named "force". Without a
comment alongside the fields, it's remaining guesswork anyway
when and how this domctl is to be used. Looking at your change
to intel_iommu_get_reserved_device_memory() the field appears
to be simply redundant.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -158,14 +158,14 @@ struct iommu_ops {
>       void (*crash_shutdown)(void);
>       void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int page_count);
>       void (*iotlb_flush_all)(struct domain *d);
> -    int (*get_reserved_device_memory)(iommu_grdm_t *, void *);
> +    int (*get_reserved_device_memory)(iommu_grdm_t *, struct domain *, void *);
>       void (*dump_p2m_table)(struct domain *d);
>   };
> 
>   void iommu_suspend(void);
>   void iommu_resume(void);
>   void iommu_crash_shutdown(void);
> -int iommu_get_reserved_device_memory(iommu_grdm_t *, void *);
> +int iommu_get_reserved_device_memory(iommu_grdm_t *, struct domain *, void *);

I don't see why these generic interfaces would need to change;
the only thing that would seem to need changing is the callback
function (and of course the context passed to it).

Jan