Re: [PATCH 03 of 14 V3] amd iommu: Add iommu emulation for hvm guest

[Wei Wang <wei.wang2@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

On 01/12/2012 12:36 PM, Jan Beulich wrote:
>>>> On 10.01.12 at 18:07, Wei Wang<wei.wang2@amd.com>  wrote:
>> +static unsigned long get_gfn_from_base_reg(uint64_t base_raw)
>> +{
>> +    uint64_t addr_lo, addr_hi, addr64;
>> +
>> +    addr_lo = iommu_get_addr_lo_from_reg(base_raw&  DMA_32BIT_MASK);
>> +    addr_hi = iommu_get_addr_hi_from_reg(base_raw>>  32);
>> +    addr64 = (addr_hi<<  32) | (addr_lo<<  PAGE_SHIFT);
> I suppose that this isn't really correct - addr_lo shouldn't really
> need any shifting, or else base_raw would be a pretty odd entity.
> I'll convert the function to use reg_to_u64() instead. While I
> won't do this, I then also wonder whether the first two operations
> could be converted to u64_to_reg(), and if so, what the purpose
> of the whole function is (it would then merely shift the input
> value to obtain a frame number)
The names might be confusing but actually iommu mmio regs do not cache 
lower 12 bit of the base addresses, so that addr_lo only contains bit 12 
- bit 31 of the lower 32 bit part. That is why 12 bit left shift is 
needed to form a fully  64 bit address. But anyway this function seems 
redundant, I attached a patch to simplify it.
Thanks,
Wei

> Jan
>
>> +
>> +    ASSERT ( addr64 != 0 );
>> +
>> +    return addr64>>  PAGE_SHIFT;
>> +}
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


[-- Attachment #2: 1.patch --]
[-- Type: text/x-patch, Size: 1846 bytes --]

# HG changeset patch
# Parent 0d4a60bf37b95b58bbae7019e0c263c556999131
# User Wei Wang <wei.wang2@amd.com>
cleanup get_gfn_from_base_reg() function.

Signed-off-by: Wei Wang <wei.wang2@amd.com>

diff -r 0d4a60bf37b9 -r 87ab207e4833 xen/drivers/passthrough/amd/iommu_guest.c
--- a/xen/drivers/passthrough/amd/iommu_guest.c	Mon Jan 16 09:55:05 2012 +0100
+++ b/xen/drivers/passthrough/amd/iommu_guest.c	Mon Jan 16 10:15:39 2012 +0100
@@ -121,16 +121,9 @@ static unsigned int host_domid(struct do
 
 static unsigned long get_gfn_from_base_reg(uint64_t base_raw)
 {
-    struct mmio_reg reg;
-    uint64_t addr64;
-
-    reg.lo = iommu_get_addr_lo_from_reg(base_raw & DMA_32BIT_MASK);
-    reg.hi = iommu_get_addr_hi_from_reg(base_raw >> 32);
-    addr64 = reg_to_u64(reg);
-
-    ASSERT ( addr64 != 0 );
-
-    return addr64 >> PAGE_SHIFT;
+    base_raw &= ~(0xFFFULL << 52);
+    ASSERT ( base_raw != 0 );
+    return base_raw >> PAGE_SHIFT;
 }
 
 static void guest_iommu_deliver_msi(struct domain *d)
diff -r 0d4a60bf37b9 -r 87ab207e4833 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h	Mon Jan 16 09:55:05 2012 +0100
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h	Mon Jan 16 10:15:39 2012 +0100
@@ -257,16 +257,4 @@ static inline void iommu_set_addr_hi_to_
                          IOMMU_REG_BASE_ADDR_HIGH_SHIFT, reg);
 }
 
-static inline uint32_t iommu_get_addr_lo_from_reg(uint32_t reg)
-{
-    return get_field_from_reg_u32(reg, IOMMU_REG_BASE_ADDR_LOW_MASK,
-                                  IOMMU_REG_BASE_ADDR_LOW_SHIFT);
-}
-
-static inline uint32_t iommu_get_addr_hi_from_reg(uint32_t reg)
-{
-    return get_field_from_reg_u32(reg, IOMMU_REG_BASE_ADDR_HIGH_MASK,
-                                  IOMMU_REG_BASE_ADDR_HIGH_SHIFT);
-}
-
 #endif /* _ASM_X86_64_AMD_IOMMU_PROTO_H */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel