Re: [Xen-devel] [PATCH v2 01/25] arm/altp2m: Add first altp2m HVMOP stubs.

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 08/03/2016 06:54 PM, Julien Grall wrote:
> Hello Sergej,
>
> On 01/08/16 18:10, Sergej Proskurin wrote:
>> This commit moves the altp2m-related code from x86 to ARM. Functions
>> that are no yet supported notify the caller or print a BUG message
>> stating their absence.
>>
>> Also, the struct arch_domain is extended with the altp2m_active
>> attribute, representing the current altp2m activity configuration of the
>> domain.
>>
>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>> ---
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Cc: Julien Grall <julien.grall@xxxxxxx>
>> ---
>> v2: Removed altp2m command-line option: Guard through HVM_PARAM_ALTP2M.
>>     Removed not used altp2m helper stubs in altp2m.h.
>> ---
>>  xen/arch/arm/hvm.c           | 79
>> ++++++++++++++++++++++++++++++++++++++++++++
>>  xen/include/asm-arm/altp2m.h |  4 +--
>>  xen/include/asm-arm/domain.h |  3 ++
>>  3 files changed, 84 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
>> index d999bde..eb524ae 100644
>> --- a/xen/arch/arm/hvm.c
>> +++ b/xen/arch/arm/hvm.c
>> @@ -32,6 +32,81 @@
>>
>>  #include <asm/hypercall.h>
>>
>> +#include <asm/altp2m.h>
>> +
>> +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +    struct xen_hvm_altp2m_op a;
>> +    struct domain *d = NULL;
>> +    int rc = 0;
>> +
>> +    if ( copy_from_guest(&a, arg, 1) )
>> +        return -EFAULT;
>> +
>> +    if ( a.pad1 || a.pad2 ||
>> +         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
>> +         (a.cmd < HVMOP_altp2m_get_domain_state) ||
>> +         (a.cmd > HVMOP_altp2m_change_gfn) )
>> +        return -EINVAL;
>> +
>> +    d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
>> +        rcu_lock_domain_by_any_id(a.domain) :
>> rcu_lock_current_domain();
>> +
>> +    if ( d == NULL )
>> +        return -ESRCH;
>> +
>> +    if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
>> +         (a.cmd != HVMOP_altp2m_set_domain_state) &&
>> +         !d->arch.altp2m_active )
>
> Why not using altp2m_active(d) here?
>

I have already changed that within the next patch version. Thank you.

> Also this check looks quite racy. What does prevent another CPU to
> disable altp2m at the same time? How the code would behave?
>

Thank you. I will protect this part with the altp2m_lock.

Best regards,
~Sergej

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