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

[Wei Wang <wei.wang2@xxxxxxx>]

On 01/12/2012 12:36 PM, Jan Beulich wrote:
> > > > On 10.01.12 at 18:07, Wei Wang<wei.wang2@xxxxxxx>  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@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

Attachment: 1.patch
Description: 1.patch

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