Re: [WIP Patch v2 12/14] KVM: arm64: Implement KVM_CAP_MEMORY_FAULT_NOWAIT

[Oliver Upton <oliver.upton@linux.dev>]

On Fri, Mar 17, 2023 at 12:00:30PM -0700, Anish Moorthy wrote:
> On Fri, Mar 17, 2023 at 11:27 AM Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> > > +     pfn = __gfn_to_pfn_memslot(
> > > +             memslot, gfn, exit_on_memory_fault, false, NULL,
> > > +             write_fault, &writable, NULL);
> >
> > As stated before [*], this google3-esque style does not match the kernel style
> > guide. You may want to check if your work machine is setting up a G3-specific
> > editor configuration behind your back.
> >
> > [*] https://lore.kernel.org/kvm/Y+0QRsZ4yWyUdpnc@google.com/
> 
> If you're referring to the indentation, then that was definitely me.
> I'll give the style guide another readthrough before I submit the next
> version then, since checkpatch.pl doesn't seem to complain here.
> 
> > > +     if (exit_on_memory_fault && pfn == KVM_PFN_ERR_FAULT) {
> >
> > nit: I don't think the local is explicitly necessary. I still find this
> > readable:
> 
> The local was for keeping a consistent value between the two blocks of code here
> 
>     pfn = __gfn_to_pfn_memslot(
>         memslot, gfn, exit_on_memory_fault, false, NULL,
>         write_fault, &writable, NULL);
> 
>     if (exit_on_memory_fault && pfn == KVM_PFN_ERR_FAULT) {
>         // Set up vCPU exit and return 0
>     }
> 
> I wanted to avoid the possibility of causing an early
> __gfn_to_pfn_memslot exit but then not populating the vCPU exit.

Ignore me, I didn't see the other use of the local.

-- 
Thanks,
Oliver