Re: [PATCH] arm/monitor vm-events: Implement guest-request support

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 02/19/2016 03:49 PM, Stefano Stabellini wrote:
> On Thu, 18 Feb 2016, Corneliu ZUZU wrote:
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cd4da04..768ad32 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -356,6 +356,7 @@ M:	Tamas K Lengyel <tamas@tklengyel.com>
>>  S:	Supported
>>  F:	xen/common/vm_event.c
>>  F:	xen/common/mem_access.c
>> +F:	xen/common/hvm/event.c
>>  F:	xen/common/monitor.c
>>  F:	xen/arch/x86/hvm/event.c
>>  F:	xen/arch/x86/monitor.c
> 
> This needs an Ack from Tamas or Razvan

The MAINTAINERS change is fine with me. Unless Tamas has an issue with
it (which he most likely doesn't), it should be fine.

>> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
>> index 056db1a..767edd1 100644
>> --- a/xen/arch/arm/hvm.c
>> +++ b/xen/arch/arm/hvm.c
>> @@ -22,6 +22,7 @@
>>  #include <xen/errno.h>
>>  #include <xen/guest_access.h>
>>  #include <xen/sched.h>
>> +#include <xen/hvm/event.h>
>>  
>>  #include <xsm/xsm.h>
>>  
>> @@ -72,6 +73,13 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>>          break;
>>      }
>>  
>> +    case HVMOP_guest_request_vm_event:
>> +        if ( guest_handle_is_null(arg) )
>> +            hvm_event_guest_request();
>> +        else
>> +            rc = -EINVAL;
>> +        break;
>> +
>>      default:
>>      {
>>          gdprintk(XENLOG_DEBUG, "HVMOP op=%lu: not implemented\n", op);
>> diff --git a/xen/common/Makefile b/xen/common/Makefile
>> index 0d76efe..703faa5 100644
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -67,7 +67,7 @@ obj-$(xenoprof)    += xenoprof.o
>>  
>>  obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o tmem_xen.o xlat.o)
>>  
>> -subdir-$(CONFIG_X86) += hvm
>> +subdir-y += hvm
>>  
>>  subdir-$(coverage) += gcov
>>  
>> diff --git a/xen/common/hvm/Makefile b/xen/common/hvm/Makefile
>> index a464a57..11e109d 100644
>> --- a/xen/common/hvm/Makefile
>> +++ b/xen/common/hvm/Makefile
>> @@ -1 +1,2 @@
>> -obj-y += save.o
>> +obj-$(CONFIG_X86) += save.o
>> +obj-y += event.o
>> diff --git a/xen/common/hvm/event.c b/xen/common/hvm/event.c
>> new file mode 100644
>> index 0000000..28ceadc
>> --- /dev/null
>> +++ b/xen/common/hvm/event.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * xen/common/hvm/event.c
>> + *
>> + * Common hardware virtual machine event abstractions.
>> + *
>> + * Copyright (c) 2004, Intel Corporation.
>> + * Copyright (c) 2005, International Business Machines Corporation.
>> + * Copyright (c) 2008, Citrix Systems, Inc.
>> + * Copyright (c) 2016, Bitdefender S.R.L.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program; If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <xen/hvm/event.h>
>> +#include <xen/vm_event.h>
>> +#include <asm/hvm/event.h>
>> +#if CONFIG_X86
>> +#include <asm/altp2m.h>
>> +#endif
>> +
>> +int hvm_event_traps(struct vcpu *v, uint8_t sync, vm_event_request_t *req)
>> +{
>> +    int rc;
>> +    struct domain *d = v->domain;
>> +
>> +    rc = vm_event_claim_slot(d, &d->vm_event->monitor);
>> +    switch ( rc )
>> +    {
>> +    case 0:
>> +        break;
>> +    case -ENOSYS:
>> +        /*
>> +         * If there was no ring to handle the event, then
>> +         * simply continue executing normally.
>> +         */
>> +        return 1;
>> +    default:
>> +        return rc;
>> +    };
>> +
>> +    if ( sync )
>> +    {
>> +        req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
>> +        vm_event_vcpu_pause(v);
>> +    }
>> +
>> +#if CONFIG_X86
>> +    if ( altp2m_active(d) )
> 
> I would rather
> 
> #define altp2m_active(d) (0)
> 
> on ARM, removing the two ifdefs in this file.

I second that, not a big fan of the #ifdefs.


Thanks,
Razvan