Re: [Xen-devel] [PATCH 2/3] tools/libxc: Make the mem_access APIs generic

["Aravindh Puthiyaparambil (aravindp)" <aravindp@xxxxxxxxx>]

>>  int xc_mem_access_resume(xc_interface *xch, domid_t domain_id,
>> unsigned long gfn)
>
>What is this gfn parameter doing?  It isn't used in the hypercall.

This was the way it was before my change. But I agree that it is pointless and 
will get rid of it. 

>>  {
>> -    return xc_mem_event_memop(xch, domain_id,
>> -                                XENMEM_access_op_resume,
>> -                                XENMEM_access_op,
>> -                                gfn, NULL);
>> +    xen_mem_access_op_t mao;
>> +
>> +    memset(&mao, 0, sizeof(mao));
>> +    mao.op    = XENMEM_access_op_resume;
>> +    mao.domid = domain_id;
>
>Please use structure initialisation for this.
>
>xen_mem_access_op_t mao =
>{
>    .op = XENMEM_access_op_resume,
>    .domid = domain_id
>};
>
>
>
>> +
>> +    return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); }
>> +
>> +int xc_set_mem_access(xc_interface *xch,
>> +                      domid_t domain_id,
>> +                      xenmem_access_t access,
>> +                      uint64_t first_pfn,
>> +                      uint32_t nr)
>> +{
>> +    xen_mem_access_op_t mao;
>> +
>> +    mao.op     = XENMEM_access_op_set_access;
>> +    mao.domid  = domain_id;
>> +    mao.access = access;
>> +    mao.pfn    = first_pfn;
>> +    mao.nr     = nr;
>> +
>> +    return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); }
>> +
>> +int xc_get_mem_access(xc_interface *xch,
>> +                      domid_t domain_id,
>> +                      uint64_t pfn,
>> +                      xenmem_access_t *access) {
>> +    xen_mem_access_op_t mao;
>> +    int rc;
>> +
>> +    mao.op    = XENMEM_access_op_get_access;
>> +    mao.domid = domain_id;
>> +    mao.pfn   = pfn;
>> +
>> +    rc = do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao));
>> +
>> +    if ( !rc )
>> +        *access = mao.access;
>
>Surely you should only write access back in the case of success?

Ah yes! This is how the original xc_hvm_get_mem_access() was written and I was 
too judicious in my copying it over. I will fix that here.

Thanks,
Aravindh


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