Re: [Xen-devel] [PATCH V2 2/25] VIOMMU: Add irq request callback to deal with irq remapping

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Aug 18, 2017 at 03:09:55PM +0800, Lan Tianyu wrote:
> On 2017年08月17日 19:18, Wei Liu wrote:
> > On Wed, Aug 09, 2017 at 04:34:03PM -0400, Lan Tianyu wrote:
> >> This patch is to add irq request callback for platform implementation
> >> to deal with irq remapping request.
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> >> ---
> >>  xen/common/viommu.c          | 15 +++++++++
> >>  xen/include/asm-x86/viommu.h | 73 
> >> ++++++++++++++++++++++++++++++++++++++++++++
> >>  xen/include/xen/viommu.h     |  9 ++++++
> >>  3 files changed, 97 insertions(+)
> >>  create mode 100644 xen/include/asm-x86/viommu.h
> >>
> >> diff --git a/xen/common/viommu.c b/xen/common/viommu.c
> >> index a4d004d..f4d34e6 100644
> >> --- a/xen/common/viommu.c
> >> +++ b/xen/common/viommu.c
> >> @@ -198,6 +198,21 @@ int __init viommu_setup(void)
> >>      return 0;
> >>  }
> >>  
> >> +int viommu_handle_irq_request(struct domain *d, u32 viommu_id,
> >> +                              struct irq_remapping_request *request)
> >> +{
> >> +    struct viommu_info *info = &d->viommu;
> > 
> > Does this compile? This patch and the previous one don't have viommu
> > added to struct domain.
> 
> Oh. Sorry. Missed patch "VIOMMU: Add vIOMMU helper functions to create,
>  destroy and query capabilities" in this series. I just sent out and
> followed this mail. Please have a look.
> 
> > 
> >> +
> >> +    if ( viommu_id >= info->nr_viommu
> >> +         || !info->viommu[viommu_id] )
> > 
> > Join this to previous line?
> >
> 
> OK.
> 
> >> +        return -EINVAL;
> > 
> > ASSERT(info->viommu[viommu_id]->ops);
> > 
> > For extra safety.
> 
> Or check ops in the previous if?
> 

That depends on if ops can be null or not.

> > 
> >> +
> >> +    if ( !info->viommu[viommu_id]->ops->handle_irq_request )
> >> +        return -EINVAL;
> >> +
> >> +    return info->viommu[viommu_id]->ops->handle_irq_request(d, request);
> >> +}
> >> +
> >>  /*
> >>   * Local variables:
> >>   * mode: C
> >> diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h
> >> new file mode 100644
> >> index 0000000..51bda72
> >> --- /dev/null
> >> +++ b/xen/include/asm-x86/viommu.h
> >> @@ -0,0 +1,73 @@
> >> +/*
> >> + * include/asm-x86/viommu.h
> >> + *
> >> + * Copyright (c) 2017 Intel Corporation.
> >> + * Author: Lan Tianyu <tianyu.lan@xxxxxxxxx> 
> >> + *
> >> + * 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/>.
> >> + *
> >> + */
> >> +#ifndef __ARCH_X86_VIOMMU_H__
> >> +#define __ARCH_X86_VIOMMU_H__
> >> +
> > 
> > Is a corresponding ARM header needed? Given viommu is common code.
> 
> I added such ARM header file in previous version but Julien hope vIOMMU
> should be disabled for ARM because ARM doesn't support vIOMMU so far.
> 

If that's what he wanted, sure.

Please build test ARM as well. You can do so using travis-ci.

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