From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v5] x86/p2m: use large pages for MMIO mappings Date: Wed, 27 Jan 2016 14:51:03 +0000 Message-ID: <56A8D957.5020501@citrix.com> References: <56A25C0602000078000CA367@prv-mh.provo.novell.com> <1453724207.4320.137.camel@citrix.com> <56A6371802000078000CAA6B@prv-mh.provo.novell.com> <1453730752.4320.164.camel@citrix.com> <56A63C4002000078000CAAA7@prv-mh.provo.novell.com> <1453731704.4320.173.camel@citrix.com> <56A658FE02000078000CAC3D@prv-mh.provo.novell.com> <56A8B8C2.5010905@citrix.com> <56A8D61202000078000CB8EF@prv-mh.provo.novell.com> <56A8D401.6080100@citrix.com> <56A8E50602000078000CB9CD@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aORRM-000283-3b for xen-devel@lists.xenproject.org; Wed, 27 Jan 2016 14:51:24 +0000 In-Reply-To: <56A8E50602000078000CB9CD@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Kevin Tian , Wei Liu , Ian Campbell , Stefano Stabellini , George Dunlap , Tim Deegan , Ian Jackson , Jun Nakajima , xen-devel , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 27/01/16 14:40, Jan Beulich wrote: > >>>>> int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn, >>>>> - p2m_access_t access) >>>>> + unsigned int order, p2m_access_t access) >>>>> { >>>>> - return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct, access); >>>>> + if ( order && >>>>> + rangeset_overlaps_range(mmio_ro_ranges, mfn_x(mfn), >>>>> + mfn_x(mfn) + (1UL << order) - 1) && >>>>> + !rangeset_contains_range(mmio_ro_ranges, mfn_x(mfn), >>>>> + mfn_x(mfn) + (1UL << order) - 1) ) >>>>> + return order; >>>> Should this not be a hard error? Even retrying with a lower order is >>>> going fail. >>> Why? The latest when order == 0, rangeset_overlaps_range() >>> will return the same as rangeset_contains_range(), and hence >>> the condition above will always be false (one of the two reasons >>> for checking order first here). >> It isn't the order check which is an issue. >> >> One way or another, if the original (mfn/order) fails the rangeset >> checks, the overall call is going to fail, but it will be re-executed >> repeatedly with an order decreasing to 0. Wouldn't it be better just to >> short-circuit this back&forth? > But this won't necessarily go down to order 0. Short-circuiting > would mean taking PAGE_ORDER_2M and PAGE_ORDER_1G into > account here, which would imo severely hamper readability. Even when this check starts passing, the subsequent set_typed_p2m_entry() will fail for writeable mappings, after having constructed small pages up to the boundary of the RO region. > >> Relatedly, is there actually anything wrong with making a superpage >> read-only mapping over some scattered read-only 4K pages? > I'm afraid I don't understand: "scattered pages" and "superpage > mapping" don't seem to fit together for me. If there is a single 4K page in the RO region, and the caller attempts to create a RO 2M superpage which includes the 4K region, these checks will force the use of 4K mappings even though the 2M mapping would be fine. ~Andrew