Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Isaila Alexandru <aisaila@bitdefender.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Nakajima, Jun" <jun.nakajima@intel.com>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH V6] x86/altp2m: Hypercall to set altp2m view visibility
Date: Fri, 27 Mar 2020 02:30:15 +0000
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D19D7ECA76@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <449a58ea-e168-6c1a-33f2-7efa0b9f5a7d@bitdefender.com>

> From: Isaila Alexandru <aisaila@bitdefender.com>
> Sent: Tuesday, March 24, 2020 6:46 PM
> 
> 
> Hi Kevin and sorry for the long reply time,
> 
> On 10.03.2020 04:04,  sTian, Kevin wrote:
> >> From: Alexandru Stefan ISAILA <aisaila@bitdefender.com>
> >> Sent: Tuesday, March 3, 2020 8:23 PM
> >>
> >> At this moment a guest can call vmfunc to change the altp2m view. This
> >> should be limited in order to avoid any unwanted view switch.
> >
> > I look forward to more elaboration of the motivation, especially for one
> > who doesn't track altp2m closely like me. For example, do_altp2m_op
> > mentions three modes: external, internal, coordinated. Then is this patch
> > trying to limit the view switch in all three modes or just one of them?
> > from the definition clearly external disallows guest to change any view
> > (then why do we want per-view visibility control) while the latter two
> > both allows guest to switch the view. later you noted some exception
> > with mixed (internal) mode. then is this restriction pushed just for
> > limited (coordinated) mode?
> >
> 
> As you stated, there are some exceptions with mixed (internal) mode.
> This restriction is clearly used for coordinated mode but it also
> restricts view switching in the external mode as well. I had a good
> example to start with, let's say we have one external agent in dom0 that
> uses view1 and view2 and the logic requires the switch between the
> views. At this point VMFUNC is available to the guest so with a simple
> asm code it can witch to view 0. At this time the external agent is not
> aware that the view has switched and further more view0 was not supposed
> to be in the main logic so it crashes. This example can be extended to
> any number of views. I hope it can paint a more clear picture of what
> this patch is trying to achive.

Can VMFUNC be hidden and disabled when external mode is being used?
or is it because the mode can be dynamically switched after a VM is 
launched so you have to restrict the views in this way?

> 
> > btw I'm not sure why altp2m invents two names per mode, and their
> > mapping looks a bit weird. e.g. isn't 'coordinated' mode sound more
> > like 'mixed' mode?
> 
> Yes that is true, it si a bit weird.
> 
> >
> >>
> >> The new xc_altp2m_set_visibility() solves this by making views invisible
> >> to vmfunc.
> >
> > if one doesn't want to make view visible to vmfunc, why can't he just
> > avoids registering the view at the first place? Are you aiming for a
> > scenario that dom0 may register 10 views, with 5 views visible to
> > vmfunc with the other 5 views switched by dom0 itself?
> 
> That is one scenario, another can be that dom0 has a number of views
> created and in some time it wants to be sure that only some of the views
> can be switched, saving the rest and making them visible when the time
> is right. Sure the example given up is another example.
> 

Can you update the patch description and resend? I'll take another look then.

Thanks
Kevin

      reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-03 12:23 Alexandru Stefan ISAILA
2020-03-03 15:29 ` Jan Beulich
2020-03-04 13:57 ` Alexandru Stefan ISAILA
2020-03-04 14:07   ` Jan Beulich
2020-03-04 14:12     ` Alexandru Stefan ISAILA
2020-03-04 14:28 ` Alexandru Stefan ISAILA
2020-03-10  2:04 ` Tian, Kevin
2020-03-24 10:46   ` Isaila Alexandru
2020-03-27  2:30     ` Tian, Kevin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AADFC41AFE54684AB9EE6CBC0274A5D19D7ECA76@SHSMSX104.ccr.corp.intel.com \
    --to=kevin.tian@intel.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=aisaila@bitdefender.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=jun.nakajima@intel.com \
    --cc=konrad.wilk@oracle.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git