From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH] xen/arm: Implement domain_get_maximum_gpfn Date: Tue, 9 Sep 2014 14:50:53 +0200 Message-ID: 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="===============1584081795923408191==" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XRKss-0001Ly-MF for xen-devel@lists.xenproject.org; Tue, 09 Sep 2014 12:50:58 +0000 Received: by mail-qg0-f44.google.com with SMTP id f51so5143594qge.3 for ; Tue, 09 Sep 2014 05:50:53 -0700 (PDT) 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: Julien Grall Cc: "xen-devel@lists.xenproject.org" , Tim Deegan , Ian Campbell , Stefano Stabellini , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --===============1584081795923408191== Content-Type: multipart/alternative; boundary=001a11c36122c2b7da0502a162e8 --001a11c36122c2b7da0502a162e8 Content-Type: text/plain; charset=ISO-8859-1 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. Tamas --001a11c36122c2b7da0502a162e8 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable


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

On Mon, Sep 8, 2014 at 10:43 PM, Jul= ien 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
<= span> <mailto:Ian= .Campbell@citrix.com>> wrote:

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

=A0 =A0 Needs more rationale as to why it is required for Xen Memory (do yo= u
=A0 =A0 mean xenaccess?). I assume I'll find that in the relevant threa= d once I
=A0 =A0 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 any= thing 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 f= or 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 si= de 0 is used to indicate failure. See 7ffc9779aa5120c5098d938cb88f69a1dda9a= 0fe "x86: make certain memory sub-ops return valid values" for mo= re info.

Tamas

--001a11c36122c2b7da0502a162e8-- --===============1584081795923408191== 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 --===============1584081795923408191==--