Mailing List Archive

[PATCH v4 05/18] x86/mem_sharing: don't try to unshare twice during page fault
The page was already tried to be unshared in get_gfn_type_access. If that
didn't work, then trying again is pointless. Don't try to send vm_event again
either, simply check if there is a ring or not.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
---
xen/arch/x86/hvm/hvm.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 38e9006c92..5d24ceb469 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -38,6 +38,7 @@
#include <xen/warning.h>
#include <xen/vpci.h>
#include <xen/nospec.h>
+#include <xen/vm_event.h>
#include <asm/shadow.h>
#include <asm/hap.h>
#include <asm/current.h>
@@ -1702,11 +1703,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
struct domain *currd = curr->domain;
struct p2m_domain *p2m, *hostp2m;
int rc, fall_through = 0, paged = 0;
- int sharing_enomem = 0;
vm_event_request_t *req_ptr = NULL;
bool sync = false;
unsigned int page_order;

+#ifdef CONFIG_MEM_SHARING
+ bool sharing_enomem = false;
+#endif
+
/* On Nested Virtualization, walk the guest page table.
* If this succeeds, all is fine.
* If this fails, inject a nested page fault into the guest.
@@ -1894,14 +1898,16 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
if ( p2m_is_paged(p2mt) || (p2mt == p2m_ram_paging_out) )
paged = 1;

- /* Mem sharing: unshare the page and try again */
- if ( npfec.write_access && (p2mt == p2m_ram_shared) )
+#ifdef CONFIG_MEM_SHARING
+ /* Mem sharing: if still shared on write access then its enomem */
+ if ( npfec.write_access && p2m_is_shared(p2mt) )
{
ASSERT(p2m_is_hostp2m(p2m));
- sharing_enomem = mem_sharing_unshare_page(currd, gfn);
+ sharing_enomem = true;
rc = 1;
goto out_put_gfn;
}
+#endif

/* Spurious fault? PoD and log-dirty also take this path. */
if ( p2m_is_ram(p2mt) )
@@ -1955,19 +1961,21 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
*/
if ( paged )
p2m_mem_paging_populate(currd, gfn);
+
+#ifdef CONFIG_MEM_SHARING
if ( sharing_enomem )
{
- int rv;
-
- if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 )
+ if ( !vm_event_check_ring(currd->vm_event_share) )
{
- gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare "
- "gfn %lx, ENOMEM and no helper (rc %d)\n",
- currd->domain_id, gfn, rv);
+ gprintk(XENLOG_ERR, "Domain %pd attempt to unshare "
+ "gfn %lx, ENOMEM and no helper\n",
+ currd, gfn);
/* Crash the domain */
rc = 0;
}
}
+#endif
+
if ( req_ptr )
{
if ( monitor_traps(curr, sync, req_ptr) < 0 )
--
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 05/18] x86/mem_sharing: don't try to unshare twice during page fault [ In reply to ]
On 08.01.2020 18:14, Tamas K Lengyel wrote:
> @@ -1702,11 +1703,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> struct domain *currd = curr->domain;
> struct p2m_domain *p2m, *hostp2m;
> int rc, fall_through = 0, paged = 0;
> - int sharing_enomem = 0;
> vm_event_request_t *req_ptr = NULL;
> bool sync = false;
> unsigned int page_order;
>
> +#ifdef CONFIG_MEM_SHARING
> + bool sharing_enomem = false;
> +#endif

To reduce #ifdef-ary, could you leave this alone (or convert to
bool in place, without #ifdef) and ...

> @@ -1955,19 +1961,21 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> */
> if ( paged )
> p2m_mem_paging_populate(currd, gfn);
> +
> +#ifdef CONFIG_MEM_SHARING
> if ( sharing_enomem )
> {
> - int rv;
> -
> - if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 )
> + if ( !vm_event_check_ring(currd->vm_event_share) )
> {
> - gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare "
> - "gfn %lx, ENOMEM and no helper (rc %d)\n",
> - currd->domain_id, gfn, rv);
> + gprintk(XENLOG_ERR, "Domain %pd attempt to unshare "
> + "gfn %lx, ENOMEM and no helper\n",
> + currd, gfn);
> /* Crash the domain */
> rc = 0;
> }
> }
> +#endif

... move the #ifdef inside the braces here? With this
Acked-by: Jan Beulich <jbeulich@suse.com>


Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 05/18] x86/mem_sharing: don't try to unshare twice during page fault [ In reply to ]
On Thu, Jan 16, 2020 at 7:55 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.01.2020 18:14, Tamas K Lengyel wrote:
> > @@ -1702,11 +1703,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> > struct domain *currd = curr->domain;
> > struct p2m_domain *p2m, *hostp2m;
> > int rc, fall_through = 0, paged = 0;
> > - int sharing_enomem = 0;
> > vm_event_request_t *req_ptr = NULL;
> > bool sync = false;
> > unsigned int page_order;
> >
> > +#ifdef CONFIG_MEM_SHARING
> > + bool sharing_enomem = false;
> > +#endif
>
> To reduce #ifdef-ary, could you leave this alone (or convert to
> bool in place, without #ifdef) and ...
>
> > @@ -1955,19 +1961,21 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> > */
> > if ( paged )
> > p2m_mem_paging_populate(currd, gfn);
> > +
> > +#ifdef CONFIG_MEM_SHARING
> > if ( sharing_enomem )
> > {
> > - int rv;
> > -
> > - if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 )
> > + if ( !vm_event_check_ring(currd->vm_event_share) )
> > {
> > - gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare "
> > - "gfn %lx, ENOMEM and no helper (rc %d)\n",
> > - currd->domain_id, gfn, rv);
> > + gprintk(XENLOG_ERR, "Domain %pd attempt to unshare "
> > + "gfn %lx, ENOMEM and no helper\n",
> > + currd, gfn);
> > /* Crash the domain */
> > rc = 0;
> > }
> > }
> > +#endif
>
> ... move the #ifdef inside the braces here? With this
> Acked-by: Jan Beulich <jbeulich@suse.com>

SGTM, I assume you are counting on the compiler to just get rid of the
variable when it sees its never used?

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [PATCH v4 05/18] x86/mem_sharing: don't try to unshare twice during page fault [ In reply to ]
On 16.01.2020 16:59, Tamas K Lengyel wrote:
> On Thu, Jan 16, 2020 at 7:55 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.01.2020 18:14, Tamas K Lengyel wrote:
>>> @@ -1702,11 +1703,14 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
>>> struct domain *currd = curr->domain;
>>> struct p2m_domain *p2m, *hostp2m;
>>> int rc, fall_through = 0, paged = 0;
>>> - int sharing_enomem = 0;
>>> vm_event_request_t *req_ptr = NULL;
>>> bool sync = false;
>>> unsigned int page_order;
>>>
>>> +#ifdef CONFIG_MEM_SHARING
>>> + bool sharing_enomem = false;
>>> +#endif
>>
>> To reduce #ifdef-ary, could you leave this alone (or convert to
>> bool in place, without #ifdef) and ...
>>
>>> @@ -1955,19 +1961,21 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
>>> */
>>> if ( paged )
>>> p2m_mem_paging_populate(currd, gfn);
>>> +
>>> +#ifdef CONFIG_MEM_SHARING
>>> if ( sharing_enomem )
>>> {
>>> - int rv;
>>> -
>>> - if ( (rv = mem_sharing_notify_enomem(currd, gfn, true)) < 0 )
>>> + if ( !vm_event_check_ring(currd->vm_event_share) )
>>> {
>>> - gdprintk(XENLOG_ERR, "Domain %hu attempt to unshare "
>>> - "gfn %lx, ENOMEM and no helper (rc %d)\n",
>>> - currd->domain_id, gfn, rv);
>>> + gprintk(XENLOG_ERR, "Domain %pd attempt to unshare "
>>> + "gfn %lx, ENOMEM and no helper\n",
>>> + currd, gfn);
>>> /* Crash the domain */
>>> rc = 0;
>>> }
>>> }
>>> +#endif
>>
>> ... move the #ifdef inside the braces here? With this
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> SGTM, I assume you are counting on the compiler to just get rid of the
> variable when it sees its never used?

Yes (and for un-optimized code it doesn't matter).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel