Re: [PATCH v5 05/14] KVM: X86: Implement ring-based dirty memory tracking

[Peter Xu]

On Tue, Mar 10, 2020 at 10:11:30AM -0400, Michael S. Tsirkin wrote:
> On Tue, Mar 10, 2020 at 10:09:21AM -0400, Peter Xu wrote:
> > On Tue, Mar 10, 2020 at 02:31:55AM -0400, Michael S. Tsirkin wrote:
> > > On Mon, Mar 09, 2020 at 05:35:54PM -0400, Peter Xu wrote:
> > > > I'll probably also
> > > > move KVM_DIRTY_LOG_PAGE_OFFSET==0 definition to uapi/linux/kvm.h.
> > > 
> > > 
> > > IMHO KVM_DIRTY_LOG_PAGE_OFFSET is kind of pointless anyway - 
> > > we won't be able to move data around just by changing the
> > > uapi value since userspace isn't
> > > recompiled when kernel changes ...
> > 
> > Yes I think we can even drop this KVM_DIRTY_LOG_PAGE_OFFSET==0
> > definition.  IMHO it's only a matter of whether we would like to
> > directly reference this value in the common code (e.g., for kernel
> > virt/kvm_main.c) or we want quite a few of this instead:
> > 
> > #ifdef KVM_DIRTY_LOG_PAGE_OFFSET
> > ..
> > #endif
> 
> Hmm do other arches define it to a different value?
> Maybe I'm confused.
> If they do then it makes sense.

Yes they can. So far with this series only x86 will define it to
nonzero (64). But logically other archs can define it to different
values.

We can reference this to existing offsets that we've defined already
for different archs, like KVM_COALESCED_MMIO_PAGE_OFFSET:

  - For ppc, it's defined as 1 (arch/powerpc/include/uapi/asm/kvm.h)
  - For x86, it's defined as 2 (arch/x86/include/uapi/asm/kvm.h)
  - ...

Thanks,

> 
> > I slightly prefer to not use lots of "#ifdef"s so I chose to make sure
> > it's defined.  However I've no strong opinion on this either. So I'm
> > open to change that if anyone insists with some reasons.
> > 
> > Thanks,
> > 
> > -- 
> > Peter Xu
> 

-- 
Peter Xu