Re: [Qemu-devel] [PATCH qemu 1/5] vfio: Switch from TARGET_PAGE_MASK to qemu_real_host_page_mask

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 07/14/2015 06:32 AM, Peter Crosthwaite wrote:
> On Sun, Jul 12, 2015 at 11:15 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
>> On Fri, Jul 10, 2015 at 08:43:44PM +1000, Alexey Kardashevskiy wrote:
>>> These started switching from TARGET_PAGE_MASK (hardcoded as 4K) to
>>> a real host page size:
>>> 4e51361d7 "cpu-all: complete "real" host page size API" and
>>> f7ceed190 "vfio: cpu: Use "real" page size API"
>>>
>>> This finished the transition by:
>>> - %s/TARGET_PAGE_MASK/qemu_real_host_page_mask/
>>> - %s/TARGET_PAGE_ALIGN/REAL_HOST_PAGE_ALIGN/
>>> - removing bitfield length for offsets in VFIOQuirk::data as
>>> qemu_real_host_page_mask is not a macro
>>
>> This does not make much sense to me.  f7ceed190 moved to
>> REAL_HOST_PAGE_SIZE because it's back end stuff that really depends
>> only on the host page size.
>>
>> Here you're applying a blanket change to vfio code, in particular to
>> the DMA handling code, and for DMA both the host and target page size
>> can be relevant, depending on the details of the IOMMU implementation.
>>
>
> So the multi-arch work (for which f7ceed190 preps) does have a problem
> that needs something like this. TARGET_PAGE_MASK and TARGET_PAGE_ALIGN
> do need to go away from common code, or this has to be promoted to
> cpu-specific code. Consequently, the page size is fixed to 4K for
> multi-arch and this is not a good long-term limitation. Is the IOMMU
> page size really tied to the CPU implementation? In practice this is
> going to be the case, but IOMMU and CPU should be decoupleable.
>
> If vfio needs to respect a particular (or all?) CPUs/IOMMUs page
> alignment then can we virtualise this as data rather than a macroified
> constant?
>
> uint64_t  page_align = 0;
>
> CPU_FOREACH(cpu, ...) {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
>
>      page_align = MAX(page_align, cc->page_size);
> }
>
> /* This is a little more made up ... */
> IOMMU_FOREACH(iommu, ...) {
>      page_align = MAX(page_align, iommu->page_size);
> }

This assumes that IOMMU has a constant page size, is this always true?
Cannot it be a (contigous?) set of different size chunks, for example, one 
per memory dimm?


-- 
Alexey