From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 7/7] ARM: KVM: drop use of PAGE_S2_DEVICE
Date: Tue, 28 May 2013 15:25:11 +0100 [thread overview]
Message-ID: <51A4BE47.7080009@arm.com> (raw)
In-Reply-To: <CAEDV+gJSM4EpGNV=fN+i6s2K9z6taMzmuNjVf_KyvTUvPaZAwg@mail.gmail.com>
On 28/05/13 15:16, Christoffer Dall wrote:
> On Tue, May 28, 2013 at 3:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 27/05/13 21:01, Christoffer Dall wrote:
>>> On Tue, May 14, 2013 at 4:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> At the moment, when mapping a device into Stage-2 for a guest,
>>>> we override whatever the guest uses by forcing a device memory
>>>> type in Stage-2.
>>>>
>>>> While this is not exactly wrong, this isn't really the "spirit" of
>>>> the architecture. The hardware shouldn't have to cope for a broken
>>>> guest mapping to a device as normal memory.
>>>>
>>>
>>> So I'm trying to think of a scenario where this feature in the
>>> architecture would actually be useful, and it sounds like from you
>>> guys that it's only useful to properly run a broken guest.
>>>
>>> Are we 100% sure that a malicious guest can't leverage this to break
>>> isolation? I'm thinking something along the lines of writing to a
>>> device (for example the gic virtual cpu interface) with a cached
>>> mapping. If such a write is in fact written back to cache, and not
>>> evicted from the cache before a later time, where a different VM is
>>> running, can't that adversely affect the other VM?
>>>
>>> Probably this can never happen, but I wasn't able to convince myself
>>> of this from going through the ARM ARM...?
>>
>> I think you definitely have a point here, and I completely missed that
>> case. A shared device (like the GIC virtual CPU interface) must be
>> forced to a device memory type, otherwise we cannot ensure strict
>> isolation of guests.
>>
>> I'll drop this patch from my series and add PAGE_S2_DEVICE back to the
>> arm64 port.
>>
> We still need to get rid of the USER bit in the definition, and since
> that's a purely arch/arm/* patch I assume it should go through RMK's
> tree. Will you ack the other patch?
Sure. Just also drop the call to kvm_set_s2pte_writable in
kvm_phys_addr_ioremap, which is not required now that PAGE_S2_DEVICE
implies RW. With that, you can add my Ack.
M.
--
Jazz is not dead. It just smells funny...
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <marc.zyngier@arm.com>
To: Christoffer Dall <cdall@cs.columbia.edu>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>,
"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
KVM General <kvm@vger.kernel.org>
Subject: Re: [PATCH v3 7/7] ARM: KVM: drop use of PAGE_S2_DEVICE
Date: Tue, 28 May 2013 15:25:11 +0100 [thread overview]
Message-ID: <51A4BE47.7080009@arm.com> (raw)
In-Reply-To: <CAEDV+gJSM4EpGNV=fN+i6s2K9z6taMzmuNjVf_KyvTUvPaZAwg@mail.gmail.com>
On 28/05/13 15:16, Christoffer Dall wrote:
> On Tue, May 28, 2013 at 3:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> On 27/05/13 21:01, Christoffer Dall wrote:
>>> On Tue, May 14, 2013 at 4:11 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> At the moment, when mapping a device into Stage-2 for a guest,
>>>> we override whatever the guest uses by forcing a device memory
>>>> type in Stage-2.
>>>>
>>>> While this is not exactly wrong, this isn't really the "spirit" of
>>>> the architecture. The hardware shouldn't have to cope for a broken
>>>> guest mapping to a device as normal memory.
>>>>
>>>
>>> So I'm trying to think of a scenario where this feature in the
>>> architecture would actually be useful, and it sounds like from you
>>> guys that it's only useful to properly run a broken guest.
>>>
>>> Are we 100% sure that a malicious guest can't leverage this to break
>>> isolation? I'm thinking something along the lines of writing to a
>>> device (for example the gic virtual cpu interface) with a cached
>>> mapping. If such a write is in fact written back to cache, and not
>>> evicted from the cache before a later time, where a different VM is
>>> running, can't that adversely affect the other VM?
>>>
>>> Probably this can never happen, but I wasn't able to convince myself
>>> of this from going through the ARM ARM...?
>>
>> I think you definitely have a point here, and I completely missed that
>> case. A shared device (like the GIC virtual CPU interface) must be
>> forced to a device memory type, otherwise we cannot ensure strict
>> isolation of guests.
>>
>> I'll drop this patch from my series and add PAGE_S2_DEVICE back to the
>> arm64 port.
>>
> We still need to get rid of the USER bit in the definition, and since
> that's a purely arch/arm/* patch I assume it should go through RMK's
> tree. Will you ack the other patch?
Sure. Just also drop the call to kvm_set_s2pte_writable in
kvm_phys_addr_ioremap, which is not required now that PAGE_S2_DEVICE
implies RW. With that, you can add my Ack.
M.
--
Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2013-05-28 14:25 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-14 11:11 [PATCH v3 0/7] ARM: KVM: various mmu related fixes for 3.10 Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-14 11:11 ` [PATCH v3 1/7] ARM: KVM: be more thorough when invalidating TLBs Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-28 1:53 ` Christoffer Dall
2013-05-28 1:53 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 2/7] ARM: KVM: remove dead prototype for __kvm_tlb_flush_vmid Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-28 1:54 ` Christoffer Dall
2013-05-28 1:54 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 3/7] ARM: KVM: relax cache maintainance when building page tables Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-14 13:05 ` Will Deacon
2013-05-14 13:05 ` Will Deacon
2013-05-28 2:10 ` Christoffer Dall
2013-05-28 2:10 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 4/7] ARM: KVM: use phys_addr_t instead of unsigned long long for HYP PGDs Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-28 2:11 ` Christoffer Dall
2013-05-28 2:11 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 5/7] ARM: KVM: don't special case PC when doing an MMIO Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-28 2:11 ` Christoffer Dall
2013-05-28 2:11 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 6/7] ARM: KVM: get rid of S2_PGD_SIZE Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-28 2:12 ` Christoffer Dall
2013-05-28 2:12 ` Christoffer Dall
2013-05-28 2:15 ` Christoffer Dall
2013-05-28 2:15 ` Christoffer Dall
2013-05-14 11:11 ` [PATCH v3 7/7] ARM: KVM: drop use of PAGE_S2_DEVICE Marc Zyngier
2013-05-14 11:11 ` Marc Zyngier
2013-05-27 20:01 ` Christoffer Dall
2013-05-27 20:01 ` Christoffer Dall
2013-05-28 10:11 ` Marc Zyngier
2013-05-28 10:11 ` Marc Zyngier
2013-05-28 14:16 ` Christoffer Dall
2013-05-28 14:16 ` Christoffer Dall
2013-05-28 14:25 ` Marc Zyngier [this message]
2013-05-28 14:25 ` Marc Zyngier
2013-05-28 14:29 ` Christoffer Dall
2013-05-28 14:29 ` Christoffer Dall
2013-05-21 16:07 ` [PATCH v3 0/7] ARM: KVM: various mmu related fixes for 3.10 Catalin Marinas
2013-05-21 16:07 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51A4BE47.7080009@arm.com \
--to=marc.zyngier@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.