[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 9/9] x86/P2M: relax permissions of PVH Dom0's MMIO entries


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 23 Sep 2021 13:10:40 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=QCMVIrccNaP+l9SSMJIa+u7dt+224mLkUJX64Qpo0J4=; b=MB9fzhiQUoOGM84T/Yh8Dv2+S3J9XD2A2bYZaEntVXeu6kuchELYmCGyNlSQCOpq72epZyjIaBKy+XN7SjaR9iOEsxlQxTkJMCavg3j/gT4C4wFEkEZ3D7kC63X7Gb7LFzcKHGXKYQch+8hzbM6bPhcRAS94/uBa+w0cMBom1wkeicYgqtvtGArbrx+jUh8mXNpGaWjR3V+wkAcfqvlwKiVBWkbeVlHXsL1fZ25zCIpNgDIr2s+S2e56NuNDMkwJxKqXDnjLkzuHRz+MONBtOa+DmoWVZ08cJ+JvMg8tpbuZv1Sa/ipOo3EV9JZ7L9PcVmXFeu+W/Uj7L6JUQki0pg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YzuGAKM2V1GV2HMOyck7wMXmtEaVrh1A8r6Wj5H26kJs7lrO/+Lp8fk46Fc0GyrC7sZVoSHGOZpplWZ2Muqa6l7G9G/CDGH8t5e6UC/l5ONSoDyBsM1eS+jP2TWgJM40dM75mY1AX+fLZwDYSGObEBt+8X8RyW1Q9jifTVdYWfpXOC40aS4smb3xFJKc1FmBU8P8/aVLDktGM6XrIkGecUMOVmZzu5GTfUD5xy37Ylc/sFRx1doFfKzi5Ve6wT5aWyOZJgMIGcFd1lR0gwv5ao5CGIHzkpO2R9cVzjr6npG4OPoFUtHoR3PX8e2YcQyhHs4j2pQm7eurXrCgzZPP0g==
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Thu, 23 Sep 2021 11:10:52 +0000
  • Ironport-data: A9a23:I4BrWau4KzKotsrxkMir6Z6dc+fnVIhZMUV32f8akzHdYApBsoF/q tZmKT+PM6zYNGKged0gPI20o0NSscTUn4A1HFM9qi81RHwT+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZQP0VOZigHtIQMsadUsxKbVIiGHpJZS5LwbZj29Y524PhWmthh PupyyHhEA79s9JLGjp8B5Kr8HuDa9yr5Vv0FnRnDRx6lAe2e0s9VfrzFonoR5fMeaFGH/bSe gr25OrRElU1XfsaIojNfr7TKiXmS1NJVOSEoiI+t6OK2nCuqsGuu0qS2TV1hUp/0l20c95NJ Npl6KaQaSIiAfP2lLpMDkZ7Aw9kHfNX5+qSSZS/mZT7I0zudnLtx7NlDV0sPJ1e8eFyaY1M3 aVGcnZXNEnF3r/ohuLgIgVvrp1LwM3DJoQQt2sm1TjEJf0nXYrCU+PB4towMDIY25sWR6+AN 5FxhTxHK0vnWg1rCFsrULEC29jyn1vhYyR2twfAzUYwyzeKl1EguFT3C/LKfvSaSMMTmVyXz krW8mK8DhwEOdi3zTue7mnqluLJhTn8Wo8ZCPu/7PECqF+Zy3EXCRYWfUCmuvT/gUm7M++zM GRNpHBo9/JrshX2EJ+tBHVUvUJooDZBYfpwNuYFtjuBlPuJvCWJKHIkEC9ePYlOWNANeRQm0 VqAntXMDDNpsaGIRX/1yop4vQ9eKgBOcjRfP3FsoR8tpoC5+dBu0kunosNLTfbt5uAZDw0c1 NxjQMIWvLwVkcdD/KGy51mvb9mE98WRE1ZdCuk6WAuYAuJFiGyNO9fABbvzt68owGOlor+p5 iNsdy+2trxmMH11vHbRKNjh5Znwjxp/DBXSgER0A74q/Cm39niocOh4uW8lfhwyap5fJ2W4M Sc/XD+9ArcJZxNGioctP+qM5zkCl/C8RbwJqNiOBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzK66R4YIhIf0/llKeHr5FuZdyn3xW7T6DFPjTkkX8uZLDNSH9dFvwGAbXBgzPxPjf+1u9H hc2H5bi9iizp8WkOXSIrd5PcwpaRZX5bLivw/Fqmie4ClMOMEkqCuPLwKNnfIpgnq9PkfzP8 G37UUhdoGcTT1WeQelTQnw8Or7pQ7hlqnc3YX4lMVqygiBxaoez9qYPMZAweOB/puBkyPd1S dgDetmBXasTGmiWpWxFYMmvtpFmeTSqmRmKY3ivbg8gcsMyXAfO4NLlIFfirXFcEiqtuMIii LS8zQeHE4EbTgFvAZ+OOvKixl+8p1YHn+d2UxeaK9VfYhy0ooNrNzbwnrk8JMRVcUfPwT6T1 gC3BxYEpLaS/99poYeR3a3d9tWnCepzGEZeDlL317fuOHmI5HenzK9BTP2MIWLXWlTr9fjwf u5S1fz9bqEKxQ4Yr4pmHr935qsi/N+z9aRCxwFpEXiXPVSmDrRsfiuP0cVV7/Afw7ZYvU29W 16V+8kcMrKMYZu3HFkULQsjT+KCyfBLxWWCsaVreB33tH1t4b6KcUROJB3d2iVSIYx8PJ4h3 ep86tUd7Bayi0ZyP9uL5syOG79g8pDUv30bi6wn
  • Ironport-hdrordr: A9a23:aVw5kK6/mCiJ95ndjwPXwVOBI+orL9Y04lQ7vn2ZFiY7TiXIra yTdaoguCMc6AxxZJkh8erwX5VoZUmsj6KdgLNhRotKOTOJhILGFvAB0WKP+UyEJ8S6zJ8h6U 4CSdkBNDSTNykCsS+S2mDReLxBsbr3gZxAx92ut0uFJTsaFJ2IhD0JbDpzfHcGIDWvUvECZe ahD4d81nCdUEVSSv7+KmgOXuDFqdGOvJX6YSQeDxpizAWVlzun5JPzDhDdh34lIn9y6IZn1V KAvx3y562lvf3+4hjA11XL55ATvNf60NNMCOGFl8BQADTxjQSDYphnRtS5zX0IidDqzGxvvM jHoh8mMcg2w3TNflutqR+o4AXk2CZG0Q6q9XaoxV/Y5eDpTjMzDMRMwahDdAHC1kYmtNZglI pWwmOwrfNsfFL9tRW4w+KNewBhl0Kyr3Znu/UUlWZjXYwXb6IUhZAD/XlSDIwLEEvBmc4a+d FVfYLhDcttABGnhyizhBgr/DXsZAV9Iv6+eDlDhiTPuAIm2EyQzCMjtboidzk7hdUAozQt3Z WwDk1SrsA8ciYhV9MLOA4we7rGNoXze2O/DIuzGyWuKEhVAQOHl3bIiI9FkN1CPqZ4iqcPpA ==
  • Ironport-sdr: irpx3oFTrN59n0nP8PpNYti4d7IxnlLCj96BRyCPIncQq/fnsmuLZjqqnC4bGCJ3hM+G8O9gWV S484bQQKEDc9uOEgalGBxtDqtUfArBmWFKsqpUBNbldq/7hfMY7zU32lQjmp6uN4GBIYvohRpp d/kidudfv4fp7XBRyRoHF+UdNSNbvLgrl53aEjmPRQkaH7Lgdp9I5cN8YIifhy4KFiNeZL/0Ke RJP/5Jp4xlXCqD6mNch+LTpihYK8sfM+96csly2/dkJvzj19vSj+YHL1gwrhZJNxRdRA2XnWs8 GErOWQA6LQH4hyPFYnYYg+35
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Sep 21, 2021 at 09:21:11AM +0200, Jan Beulich wrote:
> To become independent of the sequence of mapping operations, permit
> "access" to accumulate for Dom0, noting that there's not going to be an
> introspection agent for it which this might interfere with. While e.g.
> ideally only ROM regions would get mapped with X set, getting there is
> quite a bit of work. Plus the use of p2m_access_* here is abusive in the
> first place.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v3: Move last in series, for being controversial.
> v2: Split off from original patch. Accumulate all of R, W, and X.
> 
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1319,6 +1319,18 @@ static int set_typed_p2m_entry(struct do
>              return -EPERM;
>          }
>  
> +        /*
> +         * Gross bodge, to go away again rather sooner than later:
> +         *
> +         * For MMIO allow access permissions to accumulate, but only for 
> Dom0.
> +         * Since set_identity_p2m_entry() and set_mmio_p2m_entry() differ in
> +         * the way they specify "access", this will allow the ultimate result
> +         * to be independent of the sequence of operations.

Wouldn't it be better to 'fix' those operations so that they can work
together?

It's my understanding that set_identity_p2m_entry is the one that has
strong requirements regarding the access permissions, as on AMD ACPI
tables can specify how should regions be mapped.

A possible solution might be to make set_mmio_p2m_entry more tolerant
to how present mappings are handled. For once that function doesn't
let callers specify access permissions, so I would consider that if a
mapping is present on the gfn and it already points to the requested
mfn no error should be returned to the caller. At the end the 'default
access' for that gfn -> mfn relation is the one established by
set_identity_p2m_entry and shouldn't be subject to the p2m default
access.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.