On 30.08.2023 16:30, Roger Pau Monné wrote: > On Wed, Sep 12, 2018 at 03:09:35AM -0600, Jan Beulich wrote: >> The function does two translations in one go for a single guest access. >> Any failure of the first translation step (guest linear -> guest >> physical), resulting in #PF, ought to take precedence over any failure >> of the second step (guest physical -> host physical). > > If my understanding is correct, this is done so that any #PF that > would arise from the access is injected to the guest, regardless of > whether there might also be gfn -> mfn errors that would otherwise > prevent the #PF from being detected.
Yes. >> @@ -600,27 +614,28 @@ static void *hvmemul_map_linear_addr( >> goto out; >> >> case HVMTRANS_bad_gfn_to_mfn: >> - err = NULL; >> - goto out; >> + err = update_map_err(err, NULL); >> + continue; >> >> case HVMTRANS_gfn_paged_out: >> case HVMTRANS_gfn_shared: >> - err = ERR_PTR(~X86EMUL_RETRY); >> - goto out; >> + err = update_map_err(err, ERR_PTR(~X86EMUL_RETRY)); >> + continue; >> >> default: >> - goto unhandleable; >> + err = update_map_err(err, ERR_PTR(~X86EMUL_UNHANDLEABLE)); >> + continue; >> } >> >> *mfn++ = page_to_mfn(page); > > I have to admit it find it weird that since now we don't exit the loop > when HVMTRANS_bad_gfn_to_mfn is returned, the item at mfn[0] might > point to the gfn -> mfn translation for the second half of the access. > AFAICT that would happen if the first half of the access fails to > translate with an error !HVMTRANS_bad_linear_to_gfn and the second > half is successful. > > I guess it doesn't matter much, because the vmap below will be > skipped, might still be worth to add a comment. I could add one, but as you say it doesn't matter much, plus - I don't see a good place where such a comment would go. > In fact, if the first translation fails the following ones could use > the cheaper paging_gva_to_gfn(), as we no longer care to get the > underlying page, and just need to figure out whether the access would > trigger a #PF? That would be an option, yes, at the expense of (slightly) more complicated logic. Of course going that route would then also address your mfn[0] remark above, without the need for any comment. Jan
