On Wed, Jun 14, 2023 at 12:10 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> For future reference, the 80 char limit is a soft limit, and with a lot of
> subjectivity, can be breached. In this case, this...
Oh neat, I thought it looked pretty ugly too: done.
> Newline after variable declarations. Double demerits for breaking what was
> originally correct :-)
:(
>
> As mentioned in a previous mail, put this in the (one) caller. If more callers
> come along, which is highly unlikely, we can revisit that decision. Right now,
> it just adds noise, both here and in the helper.
>
> ...
>
> With my various suggestions, something like
>
> struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> int r;
>
> r = __kvm_write_guest_page(vcpu->kvm, slot, gfn, data, offset, len);
> if (r)
> kvm_handle_guest_uaccess_fault(...);
> return r;
>
So, the reason I had the logic within the helper was that it also
returns -EFAULT on gfn_to_hva_memslot() failures.
> static int __kvm_write_guest_page(struct kvm *kvm,
> struct kvm_memory_slot *memslot, gfn_t gfn,
> const void *data, int offset, int len)
> {
> int r;
> unsigned long addr;
>
> addr = gfn_to_hva_memslot(memslot, gfn);
> if (kvm_is_error_hva(addr))
> return -EFAULT;
> ...
Is it ok to catch these with an annotated efault? Strictly speaking
they don't seem to arise from a failed user access (perhaps my
definition is wrong: I'm looking at uaccess.h) so I'm not sure that
annotating them is valid.
