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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.11.14 at 07:32, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/11/7 19:08, Jan Beulich wrote:
>>>>> On 07.11.14 at 11:27, <tiejun.chen@xxxxxxxxx> wrote:
>>> --- 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))

Almost:

    copy_from_guest(pcidevs, xdsr->pcidevs, xdsr->num_pcidevs * 
sizeof(*pcidevs))

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

>>> @@ -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?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel