[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: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 23 Sep 2021 13:32:52 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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=NZMJ/ub7TgqiJT95OkT/1W1Q9Vvk3YKKElOuUf526wc=; b=CJUxWndU/+VSpELHWC2S93wdr/vk4V207KRLOLPLAXhY10BfKEwU/HEwROIaB/h9NXTDoVmFkJZ9b8CrbyhLoSChAg4bU6YBAKq8PPZvnneLUfzy/LERAVWtXz9OhIMJJ6ag1CtyReQ2OlqRx9a5tgVOEskQkHw5cLzdM0HMVj4uzFdcAyAxCzAwW5Kmqlx6JUes+LbazeUU3bA7R8DwEVeUayIokw7yb7eNtxk+ke9glyz4xPHFTaUrX5+50HekGiNS4jdTISngWnsrxiPXIQo0ba4ULd55xXQHGEZ/TqWZLxI70ym/VmnrqM7Cc3Itgj0IgFkyYJAN0547rinuzA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cpLVaqCG4dreKkV7Mz/QXY/QQg7snz60Vnf1f8C918LxYzEKaSMRa2aftHMt2k4D/ImwYOWSqZfdyu8DFhc+9kDejy4nUgI3GRaCCZbeYi1qWdFyi4vl7gWfQsgh/YNIAOA/ZUynMBlL0UfYQ5eoD5dIvxzBFyuq16JZuq8A3JqlRLeHEjlBJIQ3WAZGE5gZRqPSJ6/QsfupjOgoG0es4ZP8HEppMwyWH01WIjVk2lFoRULpSwTUEeSFW6d7vuIWa18G2fL6mOe1s4tBokb5Qko5ARXXOBKBFX4IpPR9MhX/DRupiZXVjLFuYUfKKfEJRRmE4TbyQzWvlpTQf8B28w==
  • Authentication-results: citrix.com; dkim=none (message not signed) header.d=none;citrix.com; dmarc=none action=none header.from=suse.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:33:04 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 23.09.2021 13:10, Roger Pau Monné wrote:
> 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?

Yes, but then we should do this properly by removing all abuse of

> 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.

I think further reducing access is in theory supposed to be possible.
For Dom0 all of this (including the potential of default access not
being RWX) a questionable thing though, as pointed out in earlier
discussions. After all there's no introspection (or alike) agent
supposed to be controlling Dom0.




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