From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55FECC433EF for ; Fri, 29 Oct 2021 13:25:59 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 108D261165 for ; Fri, 29 Oct 2021 13:25:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 108D261165 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=xen.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.218548.379034 (Exim 4.92) (envelope-from ) id 1mgRsu-0003xf-Fw; Fri, 29 Oct 2021 13:25:28 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 218548.379034; Fri, 29 Oct 2021 13:25:28 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mgRsu-0003xY-D0; Fri, 29 Oct 2021 13:25:28 +0000 Received: by outflank-mailman (input) for mailman id 218548; Fri, 29 Oct 2021 13:25:27 +0000 Received: from mail.xenproject.org ([104.130.215.37]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mgRst-0003xS-PY for xen-devel@lists.xenproject.org; Fri, 29 Oct 2021 13:25:27 +0000 Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mgRsr-0003V6-PL; Fri, 29 Oct 2021 13:25:25 +0000 Received: from [54.239.6.188] (helo=[192.168.17.171]) by xenbits.xenproject.org with esmtpsa (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1mgRsr-0002sS-IQ; Fri, 29 Oct 2021 13:25:25 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=xen.org; s=20200302mail; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID; bh=rTlCGHc8T5rt0QqFTbh9/k1ciodDLFsbwlS2hgNRauA=; b=fkeAMRxNuo3fao6Fnf7e0JIGNM gkpqG1R+hV4e5aynbyejoRlNUOdpIf2LOviRHqaGtOsA2g+4+g0KzHw4CQi9IH5KYN7wAv4cFvcyp yL71IY+Z7NwLdL/GufV5v0ZROLi+N3TqwwKU2ZODzQbmfoEgCQb5/7xiIIVbIHkFTI84=; Message-ID: <4531cc8e-e7d4-2803-b4e4-06d438b98089@xen.org> Date: Fri, 29 Oct 2021 14:25:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 Subject: Re: [PATCH for-4.16 v4] gnttab: allow setting max version per-domain To: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= Cc: xen-devel@lists.xenproject.org, Wei Liu , Andrew Cooper , George Dunlap , Jan Beulich , Stefano Stabellini , Anthony PERARD , Juergen Gross , Christian Lindig , David Scott , Volodymyr Babchuk , Ian Jackson References: <20211029075956.1260-1-roger.pau@citrix.com> <2e198e53-e3fb-4ea6-0c44-f326b5e5f5fc@xen.org> <09995bd2-0924-74bf-508f-5692b3250532@xen.org> From: Julien Grall In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 29/10/2021 12:04, Roger Pau Monné wrote: > Hello, Hi Roger, > On Fri, Oct 29, 2021 at 11:01:29AM +0100, Julien Grall wrote: >> >> >> On 29/10/2021 10:41, Roger Pau Monné wrote: >>> On Fri, Oct 29, 2021 at 09:58:55AM +0100, Julien Grall wrote: >>>> On 29/10/2021 08:59, Roger Pau Monne wrote: >>>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c >>>>> index e510395d08..f94f0f272c 100644 >>>>> --- a/xen/common/grant_table.c >>>>> +++ b/xen/common/grant_table.c >>>>> @@ -53,6 +53,7 @@ struct grant_table { >>>>> percpu_rwlock_t lock; >>>>> /* Lock protecting the maptrack limit */ >>>>> spinlock_t maptrack_lock; >>>>> + unsigned int max_version; >>>>> /* >>>>> * Defaults to v1. May be changed with GNTTABOP_set_version. All other >>>>> * values are invalid. >>>>> @@ -1917,11 +1918,33 @@ active_alloc_failed: >>>>> } >>>>> int grant_table_init(struct domain *d, int max_grant_frames, >>>>> - int max_maptrack_frames) >>>>> + int max_maptrack_frames, unsigned int options) >>>>> { >>>>> struct grant_table *gt; >>>>> + unsigned int max_grant_version = options & XEN_DOMCTL_GRANT_version_mask; >>>>> int ret = -ENOMEM; >>>>> + if ( max_grant_version == XEN_DOMCTL_GRANT_version_default ) >>>>> + max_grant_version = opt_gnttab_max_version; >>>>> + if ( !max_grant_version ) >>>>> + { >>>>> + dprintk(XENLOG_INFO, "%pd: invalid grant table version 0 requested\n", >>>>> + d); >>>>> + return -EINVAL; >>>>> + } >>>>> + if ( max_grant_version > opt_gnttab_max_version ) >>>>> + { >>>>> + dprintk(XENLOG_INFO, >>>>> + "%pd: requested grant version (%u) greater than supported (%u)\n", >>>>> + d, max_grant_version, opt_gnttab_max_version); >>>>> + return -EINVAL; >>>>> + } >>>>> + if ( unlikely(max_page >= PFN_DOWN(TB(16))) && is_pv_domain(d) && >>>> >>>> From my understanding, the limit for the grant table v1 is based on the page >>>> granularity used and the size of the fields. >>>> >>>> So the limit you add is valid for 4KB but not 16KB/64KB. Therefore, I think >>>> it would be better to use: >>>> >>>> 'max_page >= (1U << 32)' >>> >>> I'm slightly confused. Isn't Xen always using a 4KB page granularity, >> >> Yes. We only support 4KB today. But most of Xen is agnostic to the page >> granularity. I have actually started to look to allow 64KB/16KB page >> granularity for Xen on Arm in my spare time. >> >>> and that also applies to the grant table entries? >> The page granularity for the hypercall interface is whatever the page >> granularity Xen is using. So... > > I've somehow assumed that the current hypercall ABI was strictly tied > to 4KB pages, as that's for example already hardcoded in Linux > as XEN_PAGE_SIZE. It is a mess. Before, XEN_PAGE_SIZE was introduced, we were assuming that Linux and Xen were using the same page granularity. When I introduced the support to run guest with 16KB and 64KB, then we decided to introduce the define. Looking back at the decision, this was a mistake and we should have introduced an hypercall to fetch the ABI granularity instead. > >>> >>> I don't think it's possible to use correctly use a 16KB or 64KB page >>> as an entry for the grant table, as Xen assumes those to always be 4KB >>> based. >> >> ... if you build Xen with 16KB, then the grant table entries will be using >> 16KB. >> >> So I would like to avoid making the assumption that we are always using 4KB. >> That said, the worse that can happen is a spurious message. So this is more >> to get an accurate check. > > I don't have strong objections to using max_page >> 32, it might even > be clearer than checking against TB(16). > > It's just that the check would be wrong if we allow Xen itself to use > a different page size than the one used by the grant table interface > to the guest. With the current interface, I don't see how we can untie the hypervisor page granularity from the ABI. At least on Arm, if we were going to build Xen with 64KB page size, then we will not be able to do mapping less than 64KB in the stage-2 page-tables (aka HAP on x86). This is going to require some overhaul in the ABI to untie the page granularity from the hypervisor one. For now, if we were going to introduce page granularity 64KB, then we will automatically change the ABI to use 64KB page granularity. Therefore, I think the check would be suitable with the current code. [...] > Hm, possibly. It seems like debian-unstable arm32 gcc is building OK, > but I've got no idea if different compiler versions could complain. Thanks for the testing. It should be fine then. This would be a build failure, so it can be easily fixed afterwards if needed. Cheers, -- Julien Grall