On Fri, Sep 08, 2023, Anish Moorthy wrote: > The (gfn, data, offset, len) order of parameters is a little strange, > since "offset" actually applies to "gfn" rather than to "data". Add > docstrings to make things perfectly clear. > > Signed-off-by: Anish Moorthy <amoorthy@xxxxxxxxxx> > --- > virt/kvm/kvm_main.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 84e90ed3a134..12837416ce8a 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3014,6 +3014,9 @@ static int next_segment(unsigned long len, int offset) > return len; > } > > +/* > + * Copy 'len' bytes from guest memory at '(gfn * PAGE_SIZE) + offset' to 'data' A preceding @ is usually how kernel comments refer to paramaters, e.g. an alternative would be: /* Copy @len bytes from guest memory at '(@gfn * PAGE_SIZE) + @offset' to @data */ Though I think I find your version to be more readable. > + */ No need for a multi-line comment. > static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn, > void *data, int offset, int len) > { > @@ -3115,6 +3118,9 @@ int kvm_vcpu_read_guest_atomic(struct kvm_vcpu *vcpu, gpa_t gpa, > } > EXPORT_SYMBOL_GPL(kvm_vcpu_read_guest_atomic); > > +/* > + * Copy 'len' bytes from 'data' into guest memory at '(gfn * PAGE_SIZE) + offset' > + */ > static int __kvm_write_guest_page(struct kvm *kvm, > struct kvm_memory_slot *memslot, gfn_t gfn, > const void *data, int offset, int len) > -- > 2.42.0.283.g2d96d420d3-goog >