[Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Peter Crosthwaite]

Hi Alex and all,

I am working on a patch series to enable multiple CPU architectures to
run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
(all usermode code is a non-issue as not looking to support user-mode
multi-arch yet).

The problem I face, is that this macro depends on translate-all.c's
qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
of it will know to get the correct value depending on their CPU
specific code location. vfio is the only one I can't handle. My
knowledge on vfio is near-0, but my thinking is, since this is not
arch specific code can we instead use the raw host page alignment
rather that the CPU arch specific one?

Regards,
Peter

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Alex Williamson]

[cc +alexey]

On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
> Hi Alex and all,
> 
> I am working on a patch series to enable multiple CPU architectures to
> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
> (all usermode code is a non-issue as not looking to support user-mode
> multi-arch yet).
> 
> The problem I face, is that this macro depends on translate-all.c's
> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
> of it will know to get the correct value depending on their CPU
> specific code location. vfio is the only one I can't handle. My
> knowledge on vfio is near-0, but my thinking is, since this is not
> arch specific code can we instead use the raw host page alignment
> rather that the CPU arch specific one?

I think we could replace our use of HOST_PAGE_ALIGN with something based
only on the host's getpagesize().  I don't see that we really care about
the target page size for this usage.  Alexey, I think you're the only
arch where host and target page sizes can actually be different, do you
agree?  Thanks,

Alex

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Alexey Kardashevskiy]

On 05/27/2015 01:22 AM, Alex Williamson wrote:
> [cc +alexey]
>
> On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
>> Hi Alex and all,
>>
>> I am working on a patch series to enable multiple CPU architectures to
>> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
>> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
>> (all usermode code is a non-issue as not looking to support user-mode
>> multi-arch yet).
>>
>> The problem I face, is that this macro depends on translate-all.c's
>> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
>> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
>> of it will know to get the correct value depending on their CPU
>> specific code location. vfio is the only one I can't handle. My
>> knowledge on vfio is near-0, but my thinking is, since this is not
>> arch specific code can we instead use the raw host page alignment
>> rather that the CPU arch specific one?

What is "raw host page" here? I thought qemu_host_page_size is the one, 
where does it depend on TARGET_PAGE_SIZE?


> I think we could replace our use of HOST_PAGE_ALIGN with something based
> only on the host's getpagesize().  I don't see that we really care about
> the target page size for this usage.  Alexey, I think you're the only
> arch where host and target page sizes can actually be different, do you
> agree?  Thanks,

Strongly agree. Where it really matters (MSIX), it is already 
qemu_host_page_size and HOST_PAGE_ALIGN and I am a bit scared by that "raw 
host page alignment" :)



-- 
Alexey

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Peter Crosthwaite]

On Sun, May 31, 2015 at 6:34 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 05/27/2015 01:22 AM, Alex Williamson wrote:
>>
>> [cc +alexey]
>>
>> On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
>>>
>>> Hi Alex and all,
>>>
>>> I am working on a patch series to enable multiple CPU architectures to
>>> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
>>> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
>>> (all usermode code is a non-issue as not looking to support user-mode
>>> multi-arch yet).
>>>
>>> The problem I face, is that this macro depends on translate-all.c's
>>> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
>>> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
>>> of it will know to get the correct value depending on their CPU
>>> specific code location. vfio is the only one I can't handle. My
>>> knowledge on vfio is near-0, but my thinking is, since this is not
>>> arch specific code can we instead use the raw host page alignment
>>> rather that the CPU arch specific one?
>
>
> What is "raw host page" here? I thought qemu_host_page_size is the one,
> where does it depend on TARGET_PAGE_SIZE?
>
>

In translate-all.c:

void page_size_init(void)
{
    /* NOTE: we can always suppose that qemu_host_page_size >=
       TARGET_PAGE_SIZE */
    qemu_real_host_page_size = getpagesize();
    if (qemu_host_page_size == 0) {
        qemu_host_page_size = qemu_real_host_page_size;
    }
    if (qemu_host_page_size < TARGET_PAGE_SIZE) {
        qemu_host_page_size = TARGET_PAGE_SIZE;
    }
    qemu_host_page_mask = ~(qemu_host_page_size - 1);
}

It is clamped to be at least as big as a the TARGET_PAGE_SIZE.

>> I think we could replace our use of HOST_PAGE_ALIGN with something based
>> only on the host's getpagesize().  I don't see that we really care about
>> the target page size for this usage.  Alexey, I think you're the only
>> arch where host and target page sizes can actually be different, do you
>> agree?  Thanks,
>
>
> Strongly agree. Where it really matters (MSIX), it is already
> qemu_host_page_size and HOST_PAGE_ALIGN and I am a bit scared by that "raw
> host page alignment" :)
>

So the change would be an alternate macro based on
qemu_real_host_page_size which is what I am calling "raw host page
alignment".

Regards,
Peter

>
>
> --
> Alexey
>

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Alexey Kardashevskiy]

On 06/01/2015 04:27 AM, Peter Crosthwaite wrote:
> On Sun, May 31, 2015 at 6:34 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 05/27/2015 01:22 AM, Alex Williamson wrote:
>>>
>>> [cc +alexey]
>>>
>>> On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
>>>>
>>>> Hi Alex and all,
>>>>
>>>> I am working on a patch series to enable multiple CPU architectures to
>>>> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
>>>> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
>>>> (all usermode code is a non-issue as not looking to support user-mode
>>>> multi-arch yet).
>>>>
>>>> The problem I face, is that this macro depends on translate-all.c's
>>>> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
>>>> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
>>>> of it will know to get the correct value depending on their CPU
>>>> specific code location. vfio is the only one I can't handle. My
>>>> knowledge on vfio is near-0, but my thinking is, since this is not
>>>> arch specific code can we instead use the raw host page alignment
>>>> rather that the CPU arch specific one?
>>
>>
>> What is "raw host page" here? I thought qemu_host_page_size is the one,
>> where does it depend on TARGET_PAGE_SIZE?
>>
>>
>
> In translate-all.c:
>
> void page_size_init(void)
> {
>      /* NOTE: we can always suppose that qemu_host_page_size >=
>         TARGET_PAGE_SIZE */
>      qemu_real_host_page_size = getpagesize();
>      if (qemu_host_page_size == 0) {
>          qemu_host_page_size = qemu_real_host_page_size;
>      }
>      if (qemu_host_page_size < TARGET_PAGE_SIZE) {
>          qemu_host_page_size = TARGET_PAGE_SIZE;
>      }
>      qemu_host_page_mask = ~(qemu_host_page_size - 1);
> }
>
> It is clamped to be at least as big as a the TARGET_PAGE_SIZE.


Ah, just this one. TARGET_PAGE_SIZE is 4K for PPC64 and the actual page 
size can be 4K and 64K so that branch never works for PPC64.


>>> I think we could replace our use of HOST_PAGE_ALIGN with something based
>>> only on the host's getpagesize().  I don't see that we really care about
>>> the target page size for this usage.  Alexey, I think you're the only
>>> arch where host and target page sizes can actually be different, do you
>>> agree?  Thanks,
>>
>>
>> Strongly agree. Where it really matters (MSIX), it is already
>> qemu_host_page_size and HOST_PAGE_ALIGN and I am a bit scared by that "raw
>> host page alignment" :)
>>
>
> So the change would be an alternate macro based on
> qemu_real_host_page_size which is what I am calling "raw host page
> alignment".

Good, I like it. Cannot it be just a qemu_host_page_size? It is a bit 
confusing to have both qemu_host_page_size and qemu_real_host_page_size 
when even the first name suggests it is something non-static and sort of 
real :)



-- 
Alexey

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Peter Crosthwaite]

On Wed, Jun 3, 2015 at 4:16 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 06/01/2015 04:27 AM, Peter Crosthwaite wrote:
>>
>> On Sun, May 31, 2015 at 6:34 AM, Alexey Kardashevskiy <aik@ozlabs.ru>
>> wrote:
>>>
>>> On 05/27/2015 01:22 AM, Alex Williamson wrote:
>>>>
>>>>
>>>> [cc +alexey]
>>>>
>>>> On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
>>>>>
>>>>>
>>>>> Hi Alex and all,
>>>>>
>>>>> I am working on a patch series to enable multiple CPU architectures to
>>>>> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
>>>>> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
>>>>> (all usermode code is a non-issue as not looking to support user-mode
>>>>> multi-arch yet).
>>>>>
>>>>> The problem I face, is that this macro depends on translate-all.c's
>>>>> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
>>>>> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
>>>>> of it will know to get the correct value depending on their CPU
>>>>> specific code location. vfio is the only one I can't handle. My
>>>>> knowledge on vfio is near-0, but my thinking is, since this is not
>>>>> arch specific code can we instead use the raw host page alignment
>>>>> rather that the CPU arch specific one?
>>>
>>>
>>>
>>> What is "raw host page" here? I thought qemu_host_page_size is the one,
>>> where does it depend on TARGET_PAGE_SIZE?
>>>
>>>
>>
>> In translate-all.c:
>>
>> void page_size_init(void)
>> {
>>      /* NOTE: we can always suppose that qemu_host_page_size >=
>>         TARGET_PAGE_SIZE */
>>      qemu_real_host_page_size = getpagesize();
>>      if (qemu_host_page_size == 0) {
>>          qemu_host_page_size = qemu_real_host_page_size;
>>      }
>>      if (qemu_host_page_size < TARGET_PAGE_SIZE) {
>>          qemu_host_page_size = TARGET_PAGE_SIZE;
>>      }
>>      qemu_host_page_mask = ~(qemu_host_page_size - 1);
>> }
>>
>> It is clamped to be at least as big as a the TARGET_PAGE_SIZE.
>
>
>
> Ah, just this one. TARGET_PAGE_SIZE is 4K for PPC64 and the actual page size
> can be 4K and 64K so that branch never works for PPC64.
>
>
>>>> I think we could replace our use of HOST_PAGE_ALIGN with something based
>>>> only on the host's getpagesize().  I don't see that we really care about
>>>> the target page size for this usage.  Alexey, I think you're the only
>>>> arch where host and target page sizes can actually be different, do you
>>>> agree?  Thanks,
>>>
>>>
>>>
>>> Strongly agree. Where it really matters (MSIX), it is already
>>> qemu_host_page_size and HOST_PAGE_ALIGN and I am a bit scared by that
>>> "raw
>>> host page alignment" :)
>>>
>>
>> So the change would be an alternate macro based on
>> qemu_real_host_page_size which is what I am calling "raw host page
>> alignment".
>
>
> Good, I like it. Cannot it be just a qemu_host_page_size? It is a bit
> confusing to have both qemu_host_page_size and qemu_real_host_page_size when
> even the first name suggests it is something non-static and sort of real :)
>

Yes based on this, HOST_PAGE_ALIGN is badly named. It is really
aligning to both host and target. Some rename options:

QEMU_PAGE_ALIGN
HOST_TARGET_PAGE_ALIGN
QEMU_HOST_PAGE_ALIGN
or just unqualified PAGE_ALIGN?

Regards,
Peter

>
>
> --
> Alexey
>

Re: [Qemu-devel] VFIO use of HOST_PAGE_ALIGN

[Alexey Kardashevskiy]

On 06/04/2015 03:53 AM, Peter Crosthwaite wrote:
> On Wed, Jun 3, 2015 at 4:16 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> On 06/01/2015 04:27 AM, Peter Crosthwaite wrote:
>>>
>>> On Sun, May 31, 2015 at 6:34 AM, Alexey Kardashevskiy <aik@ozlabs.ru>
>>> wrote:
>>>>
>>>> On 05/27/2015 01:22 AM, Alex Williamson wrote:
>>>>>
>>>>>
>>>>> [cc +alexey]
>>>>>
>>>>> On Mon, 2015-05-25 at 00:48 -0700, Peter Crosthwaite wrote:
>>>>>>
>>>>>>
>>>>>> Hi Alex and all,
>>>>>>
>>>>>> I am working on a patch series to enable multiple CPU architectures to
>>>>>> run at once. It's a long story, but I have hit a snag in hw/vfio/pci.c
>>>>>> which AFAICS is the only in-tree system-mode use of HOST_PAGE_ALIGN
>>>>>> (all usermode code is a non-issue as not looking to support user-mode
>>>>>> multi-arch yet).
>>>>>>
>>>>>> The problem I face, is that this macro depends on translate-all.c's
>>>>>> qemu_host_page_size which in turn, depends on TARGET_PAGE_SIZE. I'm
>>>>>> hoping that one day, TARGET_PAGE_SIZE will be a variable and the users
>>>>>> of it will know to get the correct value depending on their CPU
>>>>>> specific code location. vfio is the only one I can't handle. My
>>>>>> knowledge on vfio is near-0, but my thinking is, since this is not
>>>>>> arch specific code can we instead use the raw host page alignment
>>>>>> rather that the CPU arch specific one?
>>>>
>>>>
>>>>
>>>> What is "raw host page" here? I thought qemu_host_page_size is the one,
>>>> where does it depend on TARGET_PAGE_SIZE?
>>>>
>>>>
>>>
>>> In translate-all.c:
>>>
>>> void page_size_init(void)
>>> {
>>>       /* NOTE: we can always suppose that qemu_host_page_size >=
>>>          TARGET_PAGE_SIZE */
>>>       qemu_real_host_page_size = getpagesize();
>>>       if (qemu_host_page_size == 0) {
>>>           qemu_host_page_size = qemu_real_host_page_size;
>>>       }
>>>       if (qemu_host_page_size < TARGET_PAGE_SIZE) {
>>>           qemu_host_page_size = TARGET_PAGE_SIZE;
>>>       }
>>>       qemu_host_page_mask = ~(qemu_host_page_size - 1);
>>> }
>>>
>>> It is clamped to be at least as big as a the TARGET_PAGE_SIZE.
>>
>>
>>
>> Ah, just this one. TARGET_PAGE_SIZE is 4K for PPC64 and the actual page size
>> can be 4K and 64K so that branch never works for PPC64.
>>
>>
>>>>> I think we could replace our use of HOST_PAGE_ALIGN with something based
>>>>> only on the host's getpagesize().  I don't see that we really care about
>>>>> the target page size for this usage.  Alexey, I think you're the only
>>>>> arch where host and target page sizes can actually be different, do you
>>>>> agree?  Thanks,
>>>>
>>>>
>>>>
>>>> Strongly agree. Where it really matters (MSIX), it is already
>>>> qemu_host_page_size and HOST_PAGE_ALIGN and I am a bit scared by that
>>>> "raw
>>>> host page alignment" :)
>>>>
>>>
>>> So the change would be an alternate macro based on
>>> qemu_real_host_page_size which is what I am calling "raw host page
>>> alignment".
>>
>>
>> Good, I like it. Cannot it be just a qemu_host_page_size? It is a bit
>> confusing to have both qemu_host_page_size and qemu_real_host_page_size when
>> even the first name suggests it is something non-static and sort of real :)
>>
>
> Yes based on this, HOST_PAGE_ALIGN is badly named. It is really
> aligning to both host and target. Some rename options:
>
> QEMU_PAGE_ALIGN
> HOST_TARGET_PAGE_ALIGN
> QEMU_HOST_PAGE_ALIGN
> or just unqualified PAGE_ALIGN?


Sorry, I have bad taste so I am not the one to ask :) I'd stick to 
HOST_PAGE_ALIGN.




-- 
Alexey