Re: [Xen-devel] [PATCH v4 08/28] x86/vvtd: Add MMIO handler for VVTD

[Chao Gao <chao.gao@xxxxxxxxx>]

On Fri, Feb 09, 2018 at 04:39:15PM +0000, Roger Pau Monné wrote:
>On Fri, Nov 17, 2017 at 02:22:15PM +0800, Chao Gao wrote:
>> This patch adds VVTD MMIO handler to deal with MMIO access.
>> 
>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>> Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
>> ---
>> v4:
>>  - only trap the register emulated in vvtd_in_range().
>>    i.e. replace PAGE_SIZE with the VVTD_MAX_OFFSET
>> ---
>>  xen/drivers/passthrough/vtd/vvtd.c | 55 
>> ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 55 insertions(+)
>> 
>> diff --git a/xen/drivers/passthrough/vtd/vvtd.c 
>> b/xen/drivers/passthrough/vtd/vvtd.c
>> index 9f76ccf..d78d878 100644
>> --- a/xen/drivers/passthrough/vtd/vvtd.c
>> +++ b/xen/drivers/passthrough/vtd/vvtd.c
>
>Now that I look at this, this is the wrong folder. This should be in
>xen/arch/x86/hvm with the rest of the emulated devices.

It is a problem we discussed in previous versions. AMD puts its vIOMMU
(iommu_guest.c) in xen/drivers/passthrough/amd/. We are following what
they did. I don't have special taste on this. If no one objects to your
suggestion, I will move it to xen/arch/x86/hvm/. Maybe create a new
intel directory since it's intel-specific and won't be used by AMD.

>
>> @@ -94,6 +94,60 @@ static inline uint64_t vvtd_get_reg_quad(const struct 
>> vvtd *vvtd, uint32_t reg)
>>      return *(uint64_t*)VVTD_REG_POS(vvtd, reg);
>>  }
>>  
>> +static void *domain_vvtd(const struct domain *d)
>> +{
>> +    if ( is_hvm_domain(d) && d->arch.hvm_domain.viommu )
>
>hvm_mmio_ops is only used by HVM guests, so the is_hvm_domain check
>here is redundant. At which point the helper can be simplified as:
>
>static struct vvtd *domain_vvtd(const struct domain *d)
>{
>    return d->arch.hvm_domain.viommu ? d->arch.hvm_domain.viommu->priv : NULL;
>}
>

Got it.

Thanks
Chao

>> +        return d->arch.hvm_domain.viommu->priv;
>> +    else
>> +        return NULL;
>> +}
>> +
>> +static int vvtd_in_range(struct vcpu *v, unsigned long addr)
>> +{
>> +    struct vvtd *vvtd = domain_vvtd(v->domain);
>const
>
>> +
>> +    if ( vvtd )
>> +        return (addr >= vvtd->base_addr) &&
>> +               (addr < vvtd->base_addr + VVTD_MAX_OFFSET);
>> +    return 0;
>> +}
>> +
>> +static int vvtd_read(struct vcpu *v, unsigned long addr,
>> +                     unsigned int len, unsigned long *pval)
>> +{
>> +    struct vvtd *vvtd = domain_vvtd(v->domain);
>const
>
>Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel