Re: [Qemu-devel] [PATCH] kvm-all: PAGE_SIZE should be real host page size
On 10 November 2015 at 16:59, Andrew Jones wrote: > On Tue, Nov 10, 2015 at 04:29:31PM +, Peter Maydell wrote: >> On 10 November 2015 at 00:23, Andrew Jones wrote: >> > -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ >> > -#define PAGE_SIZE TARGET_PAGE_SIZE >> > +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We >> > + * need to use the real host PAGE_SIZE, as that's what KVM will use. >> > + */ >> > +#define PAGE_SIZE getpagesize() >> >> Rather than defining PAGE_SIZE here (a confusing macro given >> we have several page sizes to deal with), why not just use >> getpagesize() in the one and only location where we currently >> use this macro? > > The macro is used by kernel headers that we import and include in > kvm-all.c. It's ugly, I agree, but that's how the this cookie crumbled. Oh, I see. That's pretty horrible. thanks -- PMM -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] kvm-all: PAGE_SIZE should be real host page size
On Tue, Nov 10, 2015 at 04:29:31PM +, Peter Maydell wrote:
> On 10 November 2015 at 00:23, Andrew Jones wrote:
> > Just noticed this while grepping TARGET_PAGE_SIZE for an unrelated
> > reason. I didn't use qemu_real_host_page_size as kvm_set_phys_mem()
> > does, because we'd need to make sure page_size_init() has run first.
> >
> > Signed-off-by: Andrew Jones
> > ---
> > kvm-all.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/kvm-all.c b/kvm-all.c
> > index 1bc12737723c3..de9ff5971fb3b 100644
> > --- a/kvm-all.c
> > +++ b/kvm-all.c
> > @@ -45,8 +45,10 @@
> > #include
> > #endif
> >
> > -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */
> > -#define PAGE_SIZE TARGET_PAGE_SIZE
> > +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We
> > + * need to use the real host PAGE_SIZE, as that's what KVM will use.
> > + */
> > +#define PAGE_SIZE getpagesize()
>
> Rather than defining PAGE_SIZE here (a confusing macro given
> we have several page sizes to deal with), why not just use
> getpagesize() in the one and only location where we currently
> use this macro?
The macro is used by kernel headers that we import and include in
kvm-all.c. It's ugly, I agree, but that's how the this cookie crumbled.
>
> Also, you're guaranteed that page_size_init() has been run, because
> we call that from kvm_init(), and you can't call kvm_vcpu_init()
> before kvm_init().
True, but having that dependency seemed error prone to me. If we
we some day changed when/if page_size_init is called then there
could be an issue, or if somebody did something like
kvm_init()
{
my_page_size = PAGE_SIZE;
...
page_size_init();
...
use(my_page_size)
}
things would break.
Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] kvm-all: PAGE_SIZE should be real host page size
On 10 November 2015 at 00:23, Andrew Jones wrote: > Just noticed this while grepping TARGET_PAGE_SIZE for an unrelated > reason. I didn't use qemu_real_host_page_size as kvm_set_phys_mem() > does, because we'd need to make sure page_size_init() has run first. > > Signed-off-by: Andrew Jones > --- > kvm-all.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 1bc12737723c3..de9ff5971fb3b 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -45,8 +45,10 @@ > #include > #endif > > -/* KVM uses PAGE_SIZE in its definition of COALESCED_MMIO_MAX */ > -#define PAGE_SIZE TARGET_PAGE_SIZE > +/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We > + * need to use the real host PAGE_SIZE, as that's what KVM will use. > + */ > +#define PAGE_SIZE getpagesize() Rather than defining PAGE_SIZE here (a confusing macro given we have several page sizes to deal with), why not just use getpagesize() in the one and only location where we currently use this macro? Also, you're guaranteed that page_size_init() has been run, because we call that from kvm_init(), and you can't call kvm_vcpu_init() before kvm_init(). thanks -- PMM -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
