From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH] xen: Restore p2m_access_t enum order to allow bitmask semantics Date: Thu, 10 Mar 2016 21:48:04 +0100 Message-ID: References: <1457451028-3808-1-git-send-email-malcolm.crossley@citrix.com> <56E04FBA.2030404@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6123101647395621610==" Return-path: In-Reply-To: <56E04FBA.2030404@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: George Dunlap Cc: Keir Fraser , George Dunlap , Andrew Cooper , Xen-devel , Jan Beulich , "Lengyel, Tamas" , Malcolm Crossley List-Id: xen-devel@lists.xenproject.org --===============6123101647395621610== Content-Type: multipart/alternative; boundary=001a114fa45c5893d3052db7eeba --001a114fa45c5893d3052db7eeba Content-Type: text/plain; charset=UTF-8 On Wed, Mar 9, 2016 at 5:30 PM, George Dunlap wrote: > On 08/03/16 15:30, Malcolm Crossley wrote: > > Nested hap code assumed implict bitmask semantics of the p2m_access_t > > enum prior to C/S 4c63692d7c38c5ac414fe97f8ef37b66e05abe5c > > > > The change to the enum ordering broke this assumption and caused > functional > > problems for the nested hap code. As it may be error prone to audit and > find > > all other p2m_access users assuming bitmask semantics, instead restore > the > > previous enum order and make it explict that bitmask semantics are to be > > preserved for the read, write and execute access types. > > > > Signed-off-by: Malcolm Crossley > > Looks good; but following up Jan's point, could you do a brief survey of > the places where the p2m_access values are used, and confirm that none > of them now implicitly assume that p2m_access_rwx is zero? > > (Or Tamas, can you say that you're reasonably certain nothing has now > come to depend on the value of p2m_access_rwx being zero?) > Yes, from my perspective it's all fine as checks of p2m_access values are done with the enum names and not the values directly. Tamas --001a114fa45c5893d3052db7eeba Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Wed, Mar 9, 2016 at 5:30 PM, George Dunlap <george.dunlap@ci= trix.com> wrote:
On 08/03/16 15:30, Malcolm Crossley wrote:
> Nested hap code assumed implict bitmask semant= ics of the p2m_access_t
> enum prior to C/S 4c63692d7c38c5ac414fe97f8ef37b66e05abe5c
>
> The change to the enum ordering broke this assumption and caused funct= ional
> problems for the nested hap code. As it may be error prone to audit an= d find
> all other p2m_access users assuming bitmask semantics, instead restore= the
> previous enum order and make it explict that bitmask semantics are to = be
> preserved for the read, write and execute access types.
>
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>

Looks good; but following up Jan's point, could you do a brief s= urvey of
the places where the p2m_access values are used, and confirm that none
of them now implicitly assume that p2m_access_rwx is zero?

(Or Tamas, can you say that you're reasonably certain nothing has now come to depend on the value of p2m_access_rwx being zero?)
=

Yes, from my perspective it's a= ll fine as checks of p2m_access values are done with the enum names and not= the values directly.

Tamas
--001a114fa45c5893d3052db7eeba-- --===============6123101647395621610== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============6123101647395621610==--