Re: [PATCH] x86/pv: Inject #GP for implicit grant unmaps

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.07.2022 13:51, Andrew Cooper wrote:
> On 26/07/2022 07:29, Jan Beulich wrote:
>> On 25.07.2022 19:50, Andrew Cooper wrote:
>>> This is a debug behaviour to identify buggy kernels.  Crashing the domain is
>>> the most unhelpful thing to do, because it discards the relevant context.
>>>
>>> Instead, inject #GP[0] like other permission errors in x86.  In particular,
>>> this lets the kernel provide a backtrace that's actually helpful to a
>>> developer trying to figure out what's going wrong.
>>>
>>> As a bugfix, this always injects #GP[0] to current, not l1e_owner.  It is 
>>> not
>>> l1e_owner's fault if dom0 using superpowers triggers an implicit unmap.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> Albeit preferably with ...
>>
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -1232,7 +1232,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct 
>>> domain *l1e_owner)
>>>          gdprintk(XENLOG_WARNING,
>>>                   "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
>>>                   l1e_get_intpte(l1e));
>>> -        domain_crash(l1e_owner);
>>> +        pv_inject_hw_exception(TRAP_gp_fault, 0);
>>>      }
>>>  #endif
>> ... the gdprintk() adjusted to also log l1e_owner.
> 
> Ok, how about this incremental?
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index b3393385ffb6..74054fb5f4ee 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1229,9 +1229,9 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct
> domain *l1e_owner)
>      if ( (l1e_get_flags(l1e) & _PAGE_GNTTAB) &&
>           !l1e_owner->is_shutting_down && !l1e_owner->is_dying )
>      {
> -        gdprintk(XENLOG_WARNING,
> -                 "Attempt to implicitly unmap a granted PTE %" PRIpte "\n",
> -                 l1e_get_intpte(l1e));
> +        gprintk(XENLOG_WARNING,
> +                "Attempt to implicitly %pd's gntmap PTE %" PRIpte "\n",
> +                l1e_owner, l1e_get_intpte(l1e));

DYM to drop "unmap"? With it restored (or anything similar to that
effect), fine with me.

Jan