[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [v7][RFC][PATCH 06/13] hvmloader/ram: check if guest memory is out of reserved device memory maps
On 2014/11/7 19:08, Jan Beulich wrote:
On 07.11.14 at 11:27, <tiejun.chen@xxxxxxxxx> wrote:
Are you saying Xen restrict some BDFs specific to emulate some devices?
But I don't see these associated codes.
I didn't say so. All I said that some of the SBDFs are being used by
them.
--- 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).
I'm not 100% sure if we can call current->domain in all scenarios. If
you can help me confirm this I'd really like to remove this change :)
Now I assume this should be true as follows:
Which is wrong, and not what I said. Instead you should pass the
domain as part of the context that gets made available to the
callback function.
Okay I will try to go there.
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1540,6 +1540,34 @@ int iommu_do_pci_domctl(
}
break;
+ case XEN_DOMCTL_set_rdm:
+ {
+ struct xen_domctl_set_rdm *xdsr = &domctl->u.set_rdm;
+ struct xen_guest_pcidev_info *pcidevs;
+ int i;
+
+ pcidevs = xmalloc_array(xen_guest_pcidev_info_t,
+ domctl->u.set_rdm.num_pcidevs);
+ if ( pcidevs == NULL )
+ {
+ return -ENOMEM;
+ }
+
+ for ( i = 0; i < xdsr->num_pcidevs; ++i )
+ {
+ if ( __copy_from_guest_offset(pcidevs, xdsr->pcidevs, i, 1) )
+ {
+ xfree(pcidevs);
+ return -EFAULT;
+ }
+ }
I don't see the need for a loop here. And you definitely can't use the
double-underscore-prefixed variant the way you do.
Do you mean this line?
copy_from_guest_offset(pcidevs, xdsr->pcidevs, 0,
xdsr->num_pcidevs*sizeof(xen_guest_pcidev_info_t))
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -90,6 +90,10 @@ struct hvm_domain {
/* Cached CF8 for guest PCI config cycles */
uint32_t pci_cf8;
+ uint32_t pci_rdmforce;
I still don't see why this is a uint32_t.
How about bool_t?
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -484,6 +484,24 @@ 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);
+struct xen_guest_pcidev_info {
+ uint8_t func;
+ uint8_t dev;
+ uint8_t bus;
+ int domain;
+ int rdmforce;
+};
Please see struct physdev_pci_device_add for how to properly and
space efficiently express what you need. And of course I'd expect
Yes. Actually I ever considered this point but I just think we may need
to keep a complete set of fields.
You're right and anywhere what we should do is focusing on on-demand.
the code to actually use all fields you specify here - neither domain
(which really ought to be named segment if it is what I think it is
meant to be) nor rdmforce are being used anywhere. Plus - again -
just "force" would be sufficient as a name, provided the field is
needed at all.
Okay I can use 'force' directly.
In Xen side we have 'bool_t', but we have 'bool' in tools side. So how
to define this in xen/include/public/domctl.h?
+struct xen_domctl_set_rdm {
+ uint32_t pci_rdmforce;
Same comment as on the field added to "struct hvm_domain".
Ditto.
+ uint32_t num_pcidevs;
+ XEN_GUEST_HANDLE(xen_guest_pcidev_info_t) pcidevs;
Did you _at all_ look at any of the other domctls when adding this?
There's not a single use of XEN_GUEST_HANDLE() in the whole file.
Looks I should do this,
XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
@@ -1118,7 +1137,8 @@ struct xen_domctl {
struct xen_domctl_gdbsx_domstatus gdbsx_domstatus;
struct xen_domctl_vnuma vnuma;
struct xen_domctl_psr_cmt_op psr_cmt_op;
- uint8_t pad[128];
+ struct xen_domctl_set_rdm set_rdm;
+ uint8_t pad[112];
Why are you altering the padding size here?
As I understand we should shrink this pad when we introduce new filed,
shouldn't we?
Thanks
Tiejun
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
