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

["Chen, Tiejun" <tiejun.chen@intel.com>]

>> Do you mean this line?
>>
>> copy_from_guest_offset(pcidevs, xdsr->pcidevs, 0,
>> xdsr->num_pcidevs*sizeof(xen_guest_pcidev_info_t))
>
> Almost:
>
>      copy_from_guest(pcidevs, xdsr->pcidevs, xdsr->num_pcidevs * sizeof(*pcidevs))

Fixed.

>
>>>> --- 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?
>
> Exactly.
>
>> 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?
>
> Have a structure field named e.g. "flags" and a #define consuming
> exactly one bit of it. Just like it's being done everywhere else.

Like this?

  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 bus;
+    uint8_t devfn;
+    struct {
+        uint32_t    force           : 1,
+                    reserved        : 31;
+    }flags;
+};
+typedef struct xen_guest_pcidev_info xen_guest_pcidev_info_t;
+DEFINE_XEN_GUEST_HANDLE(xen_guest_pcidev_info_t);
+/* Control whether/how we check and reserve device memory. */
+struct xen_domctl_set_rdm {
+    struct {
+        uint32_t    force           : 1,
+                    reserved        : 31;
+    }flags;
+    uint32_t    num_pcidevs;
+    XEN_GUEST_HANDLE_64(xen_guest_pcidev_info_t) pcidevs;
+};
+typedef struct xen_domctl_set_rdm xen_domctl_set_rdm_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_rdm_t);
+
  /* Retrieve sibling devices infomation of machine_sbdf */
  /* XEN_DOMCTL_get_device_group */
  struct xen_domctl_get_device_group {


>
>>>> @@ -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?
>
> You realize that this is inside a union?

Sorry I didn't realize this point.

Thanks
Tiejun