From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] xen/arm: Implement domain_get_maximum_gpfn Date: Tue, 9 Sep 2014 14:09:52 +0100 Message-ID: <540EFC20.5030306@citrix.com> References: <1404226666-7949-1-git-send-email-julien.grall@linaro.org> <53BD29CD.4020204@linaro.org> <1405526542.1087.50.camel@kazak.uk.xensource.com> <5404E5D3.4010302@linaro.org> <1409733899.24834.2.camel@citrix.com> <540E14DB.5000100@linaro.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2498572521389983682==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XRLBG-00034W-Mf for xen-devel@lists.xenproject.org; Tue, 09 Sep 2014 13:09:58 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel , Julien Grall Cc: "xen-devel@lists.xenproject.org" , Tim Deegan , Ian Campbell , Stefano Stabellini , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --===============2498572521389983682== Content-Type: multipart/alternative; boundary="------------080302030601060404060608" --------------080302030601060404060608 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 09/09/14 13:50, Tamas K Lengyel wrote: > > > On Mon, Sep 8, 2014 at 10:47 PM, Tamas K Lengyel > > wrote: > > > On Mon, Sep 8, 2014 at 10:43 PM, Julien Grall > > wrote: > > Hello Tamas, > > On 03/09/14 02:00, Tamas K Lengyel wrote: > > > > > On Wed, Sep 3, 2014 at 10:44 AM, Ian Campbell > > >> wrote: > > On Mon, 2014-09-01 at 17:32 -0400, Julien Grall wrote: > > Hi Ian, > > > > On 16/07/14 12:02, Ian Campbell wrote: > > > I'd much prefer to just have the fix to > xc_dom_gnttab_hvm_seed > for ARM > > > and continue to punt on this interface until it > is actually > needed by > > > something unavoidable on the guest side (and > simultaneously > hope that > > > day never comes...). > > > > This patch is a requirement to make Xen Memory > access working on ARM. > > Could you reconsider the possibility to apply this > patch on Xen? > > Needs more rationale as to why it is required for Xen > Memory (do you > mean xenaccess?). I assume I'll find that in the > relevant thread once I > get to it? > > > It's used in a non-critical sanity check for performance > reasons, as > seen here: > https://github.com/tklengyel/xen/blob/arm_memaccess3/xen/common/mem_access.c#L75. > Without the sanity check we might attempt to set > mem_access permissions > on gpfn's that don't exist for the guest. It wouldn't > break anything to > do that but if we know beforehand that the gpfn is outside > the scope of > what the guest has we can skip the entire thing. > > > It might be better if you carry this patch on your series. > > Regards, > > -- > Julien Grall > > > Alright. > > Thanks, > Tamas > > > As a sidenote, if this patch is problematic to merge for some reason, > the current implementation still needs to change to return 0 instead > of -ENOSYS as to conform to the x86 implementation. On the x86 side 0 > is used to indicate failure. See > 7ffc9779aa5120c5098d938cb88f69a1dda9a0fe "x86: make certain memory > sub-ops return valid values" for more info. 0 does not indicate failure. It indicates the value held in the shared info field, which if 0, indicates that the domain is still being built by the toolstack. -ENOSYS is still a valid failure case. ~Andrew --------------080302030601060404060608 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 09/09/14 13:50, Tamas K Lengyel wrote:


On Mon, Sep 8, 2014 at 10:47 PM, Tamas K Lengyel <tamas.lengyel@zentific.com> wrote:

On Mon, Sep 8, 2014 at 10:43 PM, Julien Grall <julien.grall@linaro.org> wrote:
Hello Tamas,

On 03/09/14 02:00, Tamas K Lengyel wrote:



On Wed, Sep 3, 2014 at 10:44 AM, Ian Campbell <Ian.Campbell@citrix.com
 <mailto:Ian.Campbell@citrix.com>> wrote:

    On Mon, 2014-09-01 at 17:32 -0400, Julien Grall wrote:
     > Hi Ian,
     >
     > On 16/07/14 12:02, Ian Campbell wrote:
     > > I'd much prefer to just have the fix to xc_dom_gnttab_hvm_seed
    for ARM
     > > and continue to punt on this interface until it is actually
    needed by
     > > something unavoidable on the guest side (and simultaneously
    hope that
     > > day never comes...).
     >
     > This patch is a requirement to make Xen Memory access working on ARM.
     > Could you reconsider the possibility to apply this patch on Xen?

    Needs more rationale as to why it is required for Xen Memory (do you
    mean xenaccess?). I assume I'll find that in the relevant thread once I
    get to it?


It's used in a non-critical sanity check for performance reasons, as
seen here:
https://github.com/tklengyel/xen/blob/arm_memaccess3/xen/common/mem_access.c#L75.
Without the sanity check we might attempt to set mem_access permissions
on gpfn's that don't exist for the guest. It wouldn't break anything to
do that but if we know beforehand that the gpfn is outside the scope of
what the guest has we can skip the entire thing.

It might be better if you carry this patch on your series.

Regards,

--
Julien Grall

Alright.

Thanks,
Tamas

As a sidenote, if this patch is problematic to merge for some reason, the current implementation still needs to change to return 0 instead of -ENOSYS as to conform to the x86 implementation. On the x86 side 0 is used to indicate failure. See 7ffc9779aa5120c5098d938cb88f69a1dda9a0fe "x86: make certain memory sub-ops return valid values" for more info.

0 does not indicate failure.  It indicates the value held in the shared info field, which if 0, indicates that the domain is still being built by the toolstack.

-ENOSYS is still a valid failure case.

~Andrew
--------------080302030601060404060608-- --===============2498572521389983682== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============2498572521389983682==--