ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes coherent_icache_guest_page() for external L3-cache.
--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/include/asm/kvm_mmu.h +2 -0
@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)if (!icache_is_aliasing ()) {/* PIPT */ unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); }else if (!icache_is_aivivt ()) {/* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all ();
Signature · 1 line
1.7.9.5
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Hi Pranav,
Quoted 27 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */ unsigned long hva = gfn_to_hva(kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */
Quoted 1 line
__flush_icache_all();
[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 34 lines · Marc Zyngier
Hi Pranav,
Quoted 27 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */ Quoted 1 line
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
Quoted 6 lines
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Quoted 8 lines
M.--
Fast, cheap, reliable. Pick two.
_______________________________________________
kvmarm mailing list
kvmarm at lists.cs.columbia.edu
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 38 lines · Anup Patel
Quoted 37 lines · Marc Zyngier
Hi Pranav,
Quoted 27 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>---
arch/arm64/include/asm/kvm_mmu.h| 2 ++1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */ Quoted 1 line
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Quoted 6 lines
Quoted 3 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Not sure I understand at which point you really need to make things coherent here. When you assign a new ASID because there could be stale cache entries left from an old ASID run? Can't you just flush all caches when you overrun your ASID allocator?
Quoted 6 lines
Quoted 5 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Is there any difference in performance between the 2 approaches?
Alex
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 57 lines · Alexander Graf
Quoted 50 lines · Anup Patel
Quoted 42 lines · Marc Zyngier
Hi Pranav,
Quoted 30 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>---
arch/arm64/include/asm/kvm_mmu.h| 2 ++1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT */
Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 5 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.Quoted 4 lines
It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.
Quoted 5 lines
Not sure I understand at which point you really need to make things coherent here. When you assign a new ASID because there could be stale
cache entries left from an old ASID run? Can't you just flush all caches when you overrun your ASID allocator?
First, a bit of terminology:
- ASIDs are used for userspace. KVM doesn't deal with those.
- VMIDs are used for VMs. That's what KVM deals with.
The issue here is that when the MMU is disabled, the access goes straight to RAM, bypassing the caches altogether (OK not completely true, but for the sake of the argument, that's close enough).
When userspace loads the kernel into memory, the kernel is not flushed to RAM, and may sit in the L3 cache if the cache is big enough. You end-up executing garbage... My proposed fix is to let kvmtool do the flushing, as we have userspace cache management operations for this exact purpose.
Quoted 12 lines
Quoted 12 lines
Quoted 6 lines
The alternative is do it in the kernel before running any vcpu - but
that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.
Quoted 5 lines
Quoted 3 lines
If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Is there any difference in performance between the 2 approaches?
dcache clean on each and every page being mapped in stage-2, versus a one-time penalty. Should be significant indeed.
But again, I'd like to see figures. What is the impact of each of the 3 methods for various memory and cache sizes?
M.
Signature · 1 line
Who you jivin' with that Cosmik Debris?
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 5 lines · Marc Zyngier
When userspace loads the kernel into memory, the kernel is not flushed to RAM, and may sit in the L3 cache if the cache is big enough. You end-up executing garbage... My proposed fix is to let kvmtool do the flushing, as we have userspace cache management operations for this exact purpose.
Why does this issue only apply to the loaded kernel and not to the zero bytes in the rest of RAM? I know executing zeroes isn't a very useful thing to do but it should be a well defined thing.
-- PMM
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 8 lines · Peter Maydell
Quoted 6 lines · Marc Zyngier
When userspace loads the kernel into memory, the kernel is not flushed
to RAM, and may sit in the L3 cache if the cache is big enough. You end-up executing garbage... My proposed fix is to let kvmtool do the flushing, as we have userspace cache management operations for this exact purpose.
Quoted 4 lines
Why does this issue only apply to the loaded kernel and not to the zero bytes in the rest of RAM? I know executing zeroes isn't a very useful thing to do but it should be a well defined thing.
Good point, and not quite sure just yet. Probably we get a zeroed, clean page?
Anup, can you elaborate on how your L3 cache behaves?
Thanks,
M.
Signature · 1 line
Who you jivin' with that Cosmik Debris?
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Peter Maydell
Quoted 6 lines · Marc Zyngier
When userspace loads the kernel into memory, the kernel is not flushed
to RAM, and may sit in the L3 cache if the cache is big enough. You end-up executing garbage... My proposed fix is to let kvmtool do the flushing, as we have userspace cache management operations for this exact purpose.
Quoted 7 lines
Quoted 4 lines
Why does this issue only apply to the loaded kernel and not to the zero bytes in the rest of RAM? I know executing zeroes isn't a very useful thing to do but it should be a well defined thing.
Good point, and not quite sure just yet. Probably we get a zeroed, clean page?
This would apply to zeroed pages too if some Guest OS expects zeroed-out portions in Guest RAM.
Quoted 2 lines
Anup, can you elaborate on how your L3 cache behaves?
The L3-cache is transparent to Aarch64 CPUs. It is only bypassed when caching is disabled or when accessing non-cacheable pages. Currently, we cannot disclose more details about line allocation and replacement policy because APM X-Gene specs are not released.
The issue pointed out by this patch would also apply to L2-cache (i.e. L3-cache disabled) but it usually does not manifest because L2-cache is not too big (few hundred KBs) and so dirty lines don't stay for long time in L2-cache.
Quoted 10 lines
Thanks,
M.--
Who you jivin' with that Cosmik Debris?
_______________________________________________
kvmarm mailing list
kvmarm at lists.cs.columbia.edu
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelQuoted 47 lines · Anup Patel
Quoted 39 lines · Marc Zyngier
Hi Pranav,
Quoted 27 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT */
Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 5 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.
Quoted 4 lines
It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.
Well, we have it on AArch64. Why would we blindly nuke the whole cache if we can do the right thing, efficiently, on the right range?
Quoted 11 lines
Quoted 3 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.
That sounds a lot better than randomly flushing the dcache for no good reason. Most of the time, your MMU will be ON, and only the initial text/data loaded from userspace is at risk.
But the userspace version sounds definitely better, and I'd like you to evaluate it before going the kernel way.
Thanks,
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 8 lines · Marc Zyngier
Quoted 4 lines · Anup Patel
It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.Well, we have it on AArch64. Why would we blindly nuke the whole cache if we can do the right thing, efficiently, on the right range?
When exactly would userspace have to care about the cache? This patch isn't exactly clear about the circumstances. I think you'd need a really strong reason for not dealing with this in the kernel -- in general userspace KVM tools don't otherwise have to deal with cache maintenance at all.
-- PMM
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 8 lines · Peter Maydell
Quoted 7 lines · Marc Zyngier
Quoted 5 lines · Anup Patel
It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for
KVM.
Quoted 5 lines
Quoted 3 lines
Well, we have it on AArch64. Why would we blindly nuke the whole cache
if we can do the right thing, efficiently, on the right range?When exactly would userspace have to care about the cache?
Only for the initial payload, I'd expect. Unless I've missed something more crucial?
Quoted 4 lines
This patch isn't exactly clear about the circumstances. I think you'd need a really strong reason for not dealing with this in the kernel -- in general userspace KVM tools don't otherwise have to deal with cache maintenance at all.
I believe we *could* do it in the kernel, just at the expense of a lot more CPU cycles.
A possible alternative would be to use HCR.DC, but I need to have a look and see what it breaks...
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 9 lines · Marc Zyngier
Quoted 8 lines · Peter Maydell
Quoted 7 lines · Marc Zyngier
Quoted 5 lines · Anup Patel
It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for
KVM.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Well, we have it on AArch64. Why would we blindly nuke the whole cache
if we can do the right thing, efficiently, on the right range?When exactly would userspace have to care about the cache?
Only for the initial payload, I'd expect. Unless I've missed something more crucial?
What happens if the page is swapped out, is the kernel guaranteed to flush it all the way to RAM when it swaps it back in, or would KVM have to take care of that?
Quoted 6 lines
Quoted 5 lines
This patch isn't exactly clear about the circumstances. I think you'd need a really strong reason for not dealing with this in the kernel -- in general userspace KVM tools don't otherwise have to deal with cache maintenance at all.
Quoted 6 lines
I believe we *could* do it in the kernel, just at the expense of a lot more CPU cycles.
A possible alternative would be to use HCR.DC, but I need to have a look and see what it breaks...
Could we flush the cache when we fault in pages only if the guest has the MMU disabled and trap if the guest disabled the MMU and flush the whole outer dcache at that point in time?
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 18 lines · Christoffer Dall
Quoted 17 lines · Marc Zyngier
Quoted 1 line · Peter Maydell
On 14 August 2013 16:23, Marc Zyngier <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines · Anup Patel
Quoted 1 line
It seems a bit unnatural to have cache cleaning is user-space. I
am
Quoted 10 lines
Quoted 8 lines
Quoted 4 lines
sure
other architectures don't have such cache cleaning in user-space for
KVM.Well, we have it on AArch64. Why would we blindly nuke the whole cache
if we can do the right thing, efficiently, on the right range?When exactly would userspace have to care about the cache?
Quoted 8 lines
Quoted 3 lines
Only for the initial payload, I'd expect. Unless I've missed something
more crucial?What happens if the page is swapped out, is the kernel guaranteed to flush it all the way to RAM when it swaps it back in, or would KVM have
to take care of that?
I'd expect the kernel to deal with it, otherwise you'd hit the wrong data each time you swap in a page in any userspace process.
Quoted 10 lines
Quoted 10 lines
Quoted 5 lines
This patch isn't exactly clear about the circumstances. I think you'd need a really strong reason for not dealing with this in the kernel -- in general userspace KVM tools don't otherwise have to deal with cache maintenance at all.
I believe we *could* do it in the kernel, just at the expense of a lot
more CPU cycles.
Quoted 6 lines
Quoted 3 lines
A possible alternative would be to use HCR.DC, but I need to have a look and see what it breaks...
Could we flush the cache when we fault in pages only if the guest has the MMU disabled and trap if the guest disabled the MMU and flush the whole outer dcache at that point in time?
We don't need to trap the disabling of the MMU. If the guest does that, it *must* have flushed its cache to RAM already. Otherwise, it is utterly broken, virtualization or not.
What the guest doesn't expect is the initial data to sit in the cache while it hasn't set the MMU on just yet. I may have a patch for that.
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 23 lines · Marc Zyngier
Quoted 22 lines · Christoffer Dall
Quoted 20 lines · Marc Zyngier
Quoted 1 line · Peter Maydell
On 14 August 2013 16:23, Marc Zyngier <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines · Anup Patel
Quoted 1 line
It seems a bit unnatural to have cache cleaning is
user-space. I am
Quoted 10 lines
Quoted 8 lines
Quoted 4 lines
sure
other architectures don't have such cache cleaning in user-space for
KVM.Well, we have it on AArch64. Why would we blindly nuke the whole cache
if we can do the right thing, efficiently, on the right range?When exactly would userspace have to care about the cache?
Only for the initial payload, I'd expect. Unless I've missed something
more crucial?
Quoted 8 lines
Quoted 4 lines
What happens if the page is swapped out, is the kernel guaranteed to flush it all the way to RAM when it swaps it back in, or would KVM have
to take care of that?I'd expect the kernel to deal with it, otherwise you'd hit the wrong data each time you swap in a page in any userspace process.
Unless the kernel only does that for pages that are mapped executable, which it cannot know for VMs.
Admittedly I haven't looked at the code recently. Do you know?
Quoted 17 lines
Quoted 10 lines
Quoted 10 lines
Quoted 5 lines
This patch isn't exactly clear about the circumstances. I think you'd need a really strong reason for not dealing with this in the kernel -- in general userspace KVM tools don't otherwise have to deal with cache maintenance at all.
I believe we *could* do it in the kernel, just at the expense of a lot
more CPU cycles.Quoted 6 lines
Quoted 3 lines
A possible alternative would be to use HCR.DC, but I need to have a look and see what it breaks...
Could we flush the cache when we fault in pages only if the guest has the MMU disabled and trap if the guest disabled the MMU and flush the whole outer dcache at that point in time?
Quoted 8 lines
We don't need to trap the disabling of the MMU. If the guest does that, it *must* have flushed its cache to RAM already. Otherwise, it is utterly broken, virtualization or not.
What the guest doesn't expect is the initial data to sit in the cache while it hasn't set the MMU on just yet. I may have a patch for that.
Otherwise we can at least do this only when faulting in pages and the MMU is off then, correct?
Is it completely inconceivable that user space starts the guest and when some device is poked, it loads something else into memory that is going to be executed after the VM has been started, but before turning on the MMU? If not, then doing a single flush before starting the VM won't cut it.
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Marc ZyngierQuoted 49 lines · Marc Zyngier
Quoted 48 lines · Anup Patel
Quoted 43 lines · Marc Zyngier
Hi Pranav,
Quoted 28 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */ Quoted 1 line
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
Quoted 6 lines
Quoted 2 lines
It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Well, we have it on AArch64. Why would we blindly nuke the whole cache if we can do the right thing, efficiently, on the right range?
Flushing from user-space would be better only if target virtual address range is small. If user-space loads several images at different locations or large large images in guest RAM then flushing entire d-cache by set/way would be more efficient.
Quoted 15 lines
Quoted 7 lines
Quoted 5 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Quoted 6 lines
That sounds a lot better than randomly flushing the dcache for no good reason. Most of the time, your MMU will be ON, and only the initial text/data loaded from userspace is at risk.
But the userspace version sounds definitely better, and I'd like you to evaluate it before going the kernel way.
Considering the penalty, I like the approach of flushing d-cache by set/way upon first run of VCPU because its just one time penalty.
Quoted 7 lines
Thanks,
M.--
Fast, cheap, reliable. Pick two.
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelHi Anup,
Quoted 57 lines · Anup Patel
Quoted 39 lines · Marc Zyngier
Hi Pranav,
Quoted 27 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT */
Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 5 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 7 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.
Can you please give the attached patch a spin on your HW? I've boot-tested it on a model, but of course I can't really verify that it fixes your issue.
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
Thanks,
M.
Signature · 5 lines
Fast, cheap, reliable. Pick two. -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: 0001-arm64-KVM-let-the-guest-access-cacheable-memory-when.patch URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130815/52a538b0/attachment-0001.ksh>
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Hi Marc,
Quoted 69 lines · Marc Zyngier
Hi Anup,
Quoted 57 lines · Anup Patel
Quoted 37 lines · Marc Zyngier
Hi Pranav,
Quoted 28 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */ Quoted 1 line
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
Quoted 6 lines
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Quoted 6 lines
Can you please give the attached patch a spin on your HW? I've boot-tested it on a model, but of course I can't really verify that it fixes your issue.
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
Nevertheless, we will still try your patch.
Quoted 7 lines
Thanks,
M.--
Fast, cheap, reliable. Pick two.
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 101 lines · Anup Patel
Hi Marc,
Quoted 88 lines · Marc Zyngier
Hi Anup,
Quoted 69 lines · Anup Patel
Quoted 46 lines · Marc Zyngier
Hi Pranav,
Quoted 31 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT */
Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 5 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.Quoted 5 lines
The alternative is do it in the kernel before running any vcpu - but
that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be accessed as device memory.
Quoted 6 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of Stage-1, and Stage-2 overrides still apply.
Quoted 1 line
Nevertheless, we will still try your patch.
Thanks.
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 116 lines · Marc Zyngier
Quoted 99 lines · Anup Patel
Hi Marc,
Quoted 80 lines · Marc Zyngier
Hi Anup,
Quoted 66 lines · Anup Patel
Quoted 41 lines · Marc Zyngier
Hi Pranav,
Quoted 29 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT*/ Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
Quoted 7 lines
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Quoted 8 lines
Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your issue.As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be accessed as device memory.
Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 6 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of Stage-1, and Stage-2 overrides still apply.
You had mentioned that PAGE_S2_DEVICE attribute was redundant and wanted guest to decide the memory attribute. In other words, you did not want to enforce any memory attribute in Stage2.
Please refer to this patch https://patchwork.kernel.org/patch/2543201/
Quoted 11 lines
Quoted 3 lines
Nevertheless, we will still try your patch.
Thanks.
M.--
Fast, cheap, reliable. Pick two.
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 147 lines · Anup Patel
Quoted 137 lines · Marc Zyngier
Quoted 120 lines · Anup Patel
Hi Marc,
Quoted 104 lines · Marc Zyngier
Hi Anup,
Quoted 82 lines · Anup Patel
Quoted 53 lines · Marc Zyngier
Hi Pranav,
Quoted 32 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 6 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu - but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be accessed as device memory.
Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 7 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.
Quoted 6 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/
This patch has never been merged. If you carry on following the discussion, you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.html
So Stage-2 memory attributes are used, they are not going away, and they are essential to the patch I sent this morning.
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Hi Marc,
Quoted 156 lines · Marc Zyngier
Quoted 144 lines · Anup Patel
Quoted 132 lines · Marc Zyngier
Quoted 113 lines · Anup Patel
Hi Marc,
Quoted 92 lines · Marc Zyngier
Hi Anup,
Quoted 76 lines · Anup Patel
Quoted 49 lines · Marc Zyngier
Hi Pranav,
Quoted 31 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT
*/Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 5 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a "DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Quoted 8 lines
Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your issue.As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be accessed as device memory.
Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 7 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of Stage-1, and Stage-2 overrides still apply.
Quoted 5 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you did not want to enforce any memory attribute in Stage2.Please refer to this patch https://patchwork.kernel.org/patch/2543201/
This patch has never been merged. If you carry on following the discussion, you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.html
Quoted 7 lines
So Stage-2 memory attributes are used, they are not going away, and they are essential to the patch I sent this morning.
M.--
Fast, cheap, reliable. Pick two.
HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware will programme a pass-through DMA-capable device without any flushes to guest RAM (because it has not enabled MMU).
A good example of such a device would be SATA AHCI controller given to a Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware accessing this SATA AHCI controller to load kernel images from SATA disk. In this situation, we will have to hack Guest bootloader/firmware AHCI driver to explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 34 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest
will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 7 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).
Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?
Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?
Thanks,
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 214 lines · Marc Zyngier
Quoted 205 lines · Anup Patel
Hi Marc,
Quoted 183 lines · Marc Zyngier
Quoted 168 lines · Anup Patel
Quoted 158 lines · Marc Zyngier
Quoted 131 lines · Anup Patel
Hi Marc,
Quoted 112 lines · Marc Zyngier
Hi Anup,
Quoted 87 lines · Anup Patel
Quoted 55 lines · Marc Zyngier
Hi Pranav,
Quoted 35 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-taggedVIVT
*/Quoted 2 lines
/* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am sure
other architectures don't have such cache cleaning in user-space for KVM.Quoted 5 lines
The alternative is do it in the kernel before running any vcpu - but
that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate, Outer Write-Back Write-Allocate memory"
Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be accessed as device memory.
Quoted 6 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
Quoted 3 lines
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 5 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you did not want to enforce any memory attribute in Stage2.Please refer to this patch https://patchwork.kernel.org/patch/2543201/
This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to guest RAM (because it has not enabled MMU).Quoted 6 lines
A good example of such a device would be SATA AHCI controller given to a Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware accessing this SATA AHCI controller to load kernel images from SATA disk. In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).
Quoted 2 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?
Actually, this would be a typical use-case in embedded virtualization. Other devices that fit this use-case would be network controllers, security engines, or some proprietary HW not having drivers in Linux.
Quoted 11 lines
Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Thanks,
M.--
Fast, cheap, reliable. Pick two.
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Marc ZyngierQuoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 34 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest
will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 7 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).
Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?
Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 225 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 34 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest
will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 7 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?
Quoted 6 lines
Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 34 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest
will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 7 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Also, Guest Linux already does __flush_dcache_all() from __cpu_setup() at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more __flush_dcache_all() upon first run of VCPU in KVM ARM64.
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using __flush_dcache_range() upon first run of VCPU
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
3. Use an alternate version of flush_icache_range() which will flush d-cache by PoC instead of PoU. We can also ensure that coherent_icache_guest_page() function will be called upon Stage2 prefetch aborts only.
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 41 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.
When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Quoted 4 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPU
We could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Quoted 2 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
What's the difference between (2) and (1)?
Quoted 5 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 65 lines · Marc Zyngier
Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".
Rather doing __flush_dcache_range() on each page in coherent_icache_guest_page() we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use __flush_dache_range()
Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 5 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
What's the difference between (2) and (1)?
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
Quoted 7 lines
Quoted 6 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.
Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 2 lines
-Christoffer
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 286 lines · Anup Patel
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 100 lines · Anup Patel
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simple hence
we went for this.If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".Rather doing
__flush_dcache_range()on each page incoherent_icache_guest_page()we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use__flush_dache_range()Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 4 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
Quoted 8 lines
Quoted 1 line
What's the difference between (2) and (1)?
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
And we don't have such a framework in arm64? But __flush_dcache_range does nevertheless flush outer caches as well?
Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.
Quoted 9 lines
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
And (independently):
B.1: Use __flush_dcache_range
B.2: Use something else + outer cache framework for arm64 B.3: Use flush_icache_range
Or do these all interleave somehow? If so, I don't understand why. Can you help?
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 330 lines · Christoffer Dall
Quoted 286 lines · Anup Patel
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 120 lines · Marc Zyngier
Hi Anup,
On 2013-08-14 15:22, Anup Patel wrote:
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simple hence
we went for this.If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've
boot-tested
it on a model, but of course I can't really verify that it fixes
your
issue.As far as I can see, it should fix it without any additional
flushing.Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".Rather doing
__flush_dcache_range()on each page incoherent_icache_guest_page()we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use__flush_dache_range()Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 4 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
Quoted 8 lines
Quoted 1 line
What's the difference between (2) and (1)?
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
And we don't have such a framework in arm64? But
__flush_dcache_rangedoes nevertheless flush outer caches as well?Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.Quoted 9 lines
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Quoted 9 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64 B.3: Useflush_icache_rangeOr do these all interleave somehow? If so, I don't understand why. Can you help?
Oh, I think I understand your point now. You mean if we use flush_cache_all before we run the vcpu, then it's not sufficient? I assumed we would simply be using __flush_dcache_area for the guest RAM regions which would flush to PoC.
For the record, I think we need a solution that also covers the case where a memory region is registered later, and I therefore prefer having this functionality in the stage-2 fault handler, where we are already taking care of similar issues (like your patch suggested).
Especially if we can limit ourselves to doing so when the guest MMU is disabled, then I really think this is going to be the least overhead and measuring the performance of this penalty vs. at first CPU execution is a bit overkill IMHO, since we are only talking about boot time for any reasonable guest (which would run with the MMU enabled for real workloads presumeably).
The only caveat is the previously discussed issue if user space loads code after the first VCPU execution, and only user space would know if that happens, which would argue for user space doing the cleaning... Hmmm.
I also still have my worries about swapping, since user space is free to map guest RAM as non-executable.
Did I miss anything here?
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 350 lines · Christoffer Dall
Quoted 330 lines · Christoffer Dall
Quoted 286 lines · Anup Patel
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 141 lines · Anup Patel
Hi Marc,
On Thu, Aug 15, 2013 at 10:22 AM, Marc Zyngier
<[email protected]>
wrote:Hi Anup,
On 2013-08-14 15:22, Anup Patel wrote:
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simple hence
we went for this.If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've
boot-tested
it on a model, but of course I can't really verify that it fixes
your
issue.As far as I can see, it should fix it without any additional
flushing.Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are
treated as "Normal Non-shareable, Inner Write-Back
Write-Allocate,
Outer Write-Back Write-Allocate memory"HCR_EL2.DC=0 means all memory access for Stage1 MMU off are
treated as "Strongly-ordered device memory"Now if Guest/VM access hardware MMIO devices directly (such as
VGIC CPU interface) with MMU off then MMIO devices will be
accessed as normal memory when HCR_EL2.DC=1.I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software
emulated devices for Guest/VM which is not true for KVM ARM or
KVM ARM64 because we use VGIC.Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM
when Stage1 MMU is off.See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".Rather doing
__flush_dcache_range()on each page incoherent_icache_guest_page()we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use__flush_dache_range()Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 4 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
Quoted 8 lines
Quoted 1 line
What's the difference between (2) and (1)?
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
And we don't have such a framework in arm64? But
__flush_dcache_rangedoes nevertheless flush outer caches as well?Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.Quoted 9 lines
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Quoted 9 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64 B.3: Useflush_icache_rangeOr do these all interleave somehow? If so, I don't understand why. Can you help?
Oh, I think I understand your point now. You mean if we use
flush_cache_allbefore we run the vcpu, then it's not sufficient? I assumed we would simply be using__flush_dcache_areafor the guest RAM regions which would flush to PoC.For the record, I think we need a solution that also covers the case where a memory region is registered later, and I therefore prefer having this functionality in the stage-2 fault handler, where we are already taking care of similar issues (like your patch suggested).
Quoted 6 lines
Especially if we can limit ourselves to doing so when the guest MMU is disabled, then I really think this is going to be the least overhead and measuring the performance of this penalty vs. at first CPU execution is a bit overkill IMHO, since we are only talking about boot time for any reasonable guest (which would run with the MMU enabled for real workloads presumeably).
Yes, currently we call coherent_icache_guest_page() upon all Stage2 translation faults but, we can be improve a bit here just like your suggestion. May be we can call coherent_icache_guest_page() only when VCPU MMU is off and we get Stage2 instruction prefetch translation fault.
Quoted 10 lines
The only caveat is the previously discussed issue if user space loads code after the first VCPU execution, and only user space would know if that happens, which would argue for user space doing the cleaning... Hmmm.
I also still have my worries about swapping, since user space is free to map guest RAM as non-executable.
Did I miss anything here?
I don't know much about Linux swapping but for the rest part we are in sync.
Quoted 2 lines
-Christoffer
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Christoffer DallQuoted 289 lines · Christoffer Dall
Quoted 288 lines · Anup Patel
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 120 lines · Marc Zyngier
Hi Anup,
On 2013-08-14 15:22, Anup Patel wrote:
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simple hence
we went for this.If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've
boot-tested
it on a model, but of course I can't really verify that it fixes
your
issue.As far as I can see, it should fix it without any additional
flushing.Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".Rather doing
__flush_dcache_range()on each page incoherent_icache_guest_page()we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use__flush_dache_range()Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 5 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
What's the difference between (2) and (1)?
Quoted 8 lines
Quoted 6 lines
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
And we don't have such a framework in arm64? But
__flush_dcache_range
Yes, for now we don't have outer-cache framework in arm64.
Quoted 1 line
does nevertheless flush outer caches as well?
Yes, __flush_dcache_range() flushes the outer cache too.
The __flush_dcache_range() works for us because it flushes (i.e. clean & invalidate) by VA upto PoC. All operation upto PoC on a cache line will force eviction of the cache line from all CPU caches (i.e. both L1 & L2) and also from external L3-cache to ensure that RAM has updated contents of cache line.
Now, the outer caches (such as L3-cache) typically provide a mechanism to flush entire L3-cache using L3-cache registers. If we have an outer-cache framework then we can flush all caches using __flush_dcache_all() + outer cache flush all.
Quoted 25 lines
Quoted 16 lines
Quoted 8 lines
Quoted 7 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.Quoted 9 lines
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 6 lines
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 5 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64
This would be __flush_dcache_all() + outer cache flush all.
Quoted 1 line
B.3: Use
flush_icache_range
Actually modified version of flush_icache_range() which uses "dc cvac" and not "dc cvau".
Quoted 5 lines
Or do these all interleave somehow? If so, I don't understand why. Can you help?
-Christoffer
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 357 lines · Anup Patel
Quoted 289 lines · Christoffer Dall
Quoted 288 lines · Anup Patel
Quoted 249 lines · Christoffer Dall
Quoted 248 lines · Anup Patel
Quoted 232 lines · Anup Patel
Quoted 230 lines · Christoffer Dall
Quoted 217 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 141 lines · Anup Patel
Hi Marc,
On Thu, Aug 15, 2013 at 10:22 AM, Marc Zyngier
<[email protected]>
wrote:Hi Anup,
On 2013-08-14 15:22, Anup Patel wrote:
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]That's definitely an issue, but I'm not sure the fix is to hit
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simple hence
we went for this.If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've
boot-tested
it on a model, but of course I can't really verify that it fixes
your
issue.As far as I can see, it should fix it without any additional
flushing.Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are
treated as "Normal Non-shareable, Inner Write-Back
Write-Allocate,
Outer Write-Back Write-Allocate memory"HCR_EL2.DC=0 means all memory access for Stage1 MMU off are
treated as "Strongly-ordered device memory"Now if Guest/VM access hardware MMIO devices directly (such as
VGIC CPU interface) with MMU off then MMIO devices will be
accessed as normal memory when HCR_EL2.DC=1.I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software
emulated devices for Guest/VM which is not true for KVM ARM or
KVM ARM64 because we use VGIC.Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM
when Stage1 MMU is off.See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 7 lines
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 8 lines
Quoted 1 line
-Christoffer
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
--Anup
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.When did anyone suggest using a cache cleaning method that doesn't apply to this case. I'm a little confused about your comment here.
Please refer last reply from Marc Z where he says:
"Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?".Rather doing
__flush_dcache_range()on each page incoherent_icache_guest_page()we could also flush entire d-cache upon first VCPU run. This only issue in flushing d-cache upon first VCPU run is that we cannot flush d-cache by set/way because as per ARM ARM all operations by set/way are upto PoU and not PoC. In presence of L3-Cache we need to flush d-cache upto PoC so we have to use__flush_dache_range()Quoted 8 lines
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPUWe could do that, but we have to ensure that no other memory regions can be added later. Is this the case? I don't remember.
Yes, memory regions being added later be problem for this option.
Quoted 5 lines
Quoted 3 lines
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
What's the difference between (2) and (1)?
Quoted 8 lines
Quoted 6 lines
Linux ARM (i.e. 32-bit port) has a framework for having outer caches (i.e. caches that are not part of the CPU but exist as separate entity between CPU and Memory for performance improvement). Using this framework we can flush all CPU caches and outer cache.
And we don't have such a framework in arm64? But
__flush_dcache_rangeYes, for now we don't have outer-cache framework in arm64.
Quoted 1 line
does nevertheless flush outer caches as well?
Yes,
__flush_dcache_range()flushes the outer cache too.The
__flush_dcache_range()works for us because it flushes (i.e. clean & invalidate) by VA upto PoC. All operation upto PoC on a cache line will force eviction of the cache line from all CPU caches (i.e. both L1 & L2) and also from external L3-cache to ensure that RAM has updated contents of cache line.Now, the outer caches (such as L3-cache) typically provide a mechanism to flush entire L3-cache using L3-cache registers. If we have an outer-cache framework then we can flush all caches using
__flush_dcache_all()+ outer cache flush all.Quoted 25 lines
Quoted 16 lines
Quoted 8 lines
Quoted 7 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.Quoted 7 lines
I'm sorry, but I'm not following this discussion. Are we not mixing a discussion along two different axis here? As I see it there are two separate issues:
(A) When/Where to flush the memory regions
(B) How to flush the memory regions, including the hardware method and the kernel software engineering aspect.Quoted 9 lines
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 6 lines
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 5 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64
Quoted 11 lines
This would be
__flush_dcache_all()+ outer cache flush all.Quoted 1 line
B.3: Use
flush_icache_rangeActually modified version of
flush_icache_range()which uses "dc cvac" and not "dc cvau".Quoted 4 lines
Or do these all interleave somehow? If so, I don't understand why. Can you help?
Hi Anup,
Thanks for the explanation, it's crystal now.
-Christoffer
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelQuoted 22 lines · Anup Patel
Quoted 10 lines · Christoffer Dall
Quoted 9 lines · Anup Patel
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 6 lines
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64This would be
__flush_dcache_all()+ outer cache flush all.
We need to be careful here since the __flush_dcache_all() operation uses cache maintenance by set/way and these are *local* to a CPU (IOW not broadcast). Do you have any guarantee that dirty cache lines don't migrate between CPUs and __flush_dcache_all() wouldn't miss them? Architecturally we don't, so this is not a safe operation that would guarantee L1 cache flushing (we probably need to revisit some of the __flush_dcache_all() calls in KVM, I haven't looked into this).
So I think we are left to the range operation where the DC ops to PoC would be enough for your L3.
An outer cache flush all is probably only needed for cpuidle/suspend (the booting part should be handled by the boot loader).
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 38 lines · Catalin Marinas
Quoted 22 lines · Anup Patel
Quoted 10 lines · Christoffer Dall
Quoted 9 lines · Anup Patel
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 6 lines
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64This would be
__flush_dcache_all()+ outer cache flush all.We need to be careful here since the
__flush_dcache_all()operation uses cache maintenance by set/way and these are *local* to a CPU (IOW not broadcast). Do you have any guarantee that dirty cache lines don't migrate between CPUs and__flush_dcache_all()wouldn't miss them? Architecturally we don't, so this is not a safe operation that would guarantee L1 cache flushing (we probably need to revisit some of the__flush_dcache_all()calls in KVM, I haven't looked into this).
Quoted 2 lines
So I think we are left to the range operation where the DC ops to PoC would be enough for your L3.
If __flush_dcache_all() is *local" to a CPU then I guess DC ops to PoC by range would be the only option.
Quoted 3 lines
An outer cache flush all is probably only needed for cpuidle/suspend (the booting part should be handled by the boot loader).
Yes, cpuidle/suspend would definitely require outer cache maintenance.
For KVM, we can avoid flushing d-cache to PoC every time in coherent_icache_guest_page() by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.
Quoted 3 lines
--
Catalin
--Anup
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 40 lines · Anup Patel
Quoted 38 lines · Catalin Marinas
Quoted 22 lines · Anup Patel
Quoted 10 lines · Christoffer Dall
Quoted 9 lines · Anup Patel
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting a VCPU user-space typically loads images to guest RAM so, in-presence of huge L3-cache (few MBs). When the VCPU starts running some of the contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush the guest RAM contents before they are accessed by first time by VCPU.
Quoted 6 lines
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64This would be
__flush_dcache_all()+ outer cache flush all.We need to be careful here since the
__flush_dcache_all()operation uses cache maintenance by set/way and these are *local* to a CPU (IOW not broadcast). Do you have any guarantee that dirty cache lines don't migrate between CPUs and__flush_dcache_all()wouldn't miss them? Architecturally we don't, so this is not a safe operation that would guarantee L1 cache flushing (we probably need to revisit some of the__flush_dcache_all()calls in KVM, I haven't looked into this).
Quoted 5 lines
Quoted 2 lines
So I think we are left to the range operation where the DC ops to PoC would be enough for your L3.
If
__flush_dcache_all()is *local" to a CPU then I guess DC ops to PoC by range would be the only option.
Yes. In the (upcoming) ARM ARMv8 there is a clear note that set/way operations to flush the whole cache must not be used for the maintenance of large buffer but only during power-down/power-up code sequences.
Quoted 8 lines
Quoted 2 lines
An outer cache flush all is probably only needed for cpuidle/suspend (the booting part should be handled by the boot loader).
Yes, cpuidle/suspend would definitely require outer cache maintenance.
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.
That's for the KVM guys to decide ;)
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelQuoted 58 lines · Anup Patel
Quoted 44 lines · Catalin Marinas
Quoted 27 lines · Anup Patel
Quoted 3 lines · Christoffer Dall
Quoted 2 lines · Anup Patel
Discussion here is about getting KVM ARM64 working in-presence of an external L3-cache (i.e. not part of CPU). Before starting
a VCPU
Quoted 1 line
Quoted 1 line
user-space typically loads images to guest RAM so, in-presence
of
Quoted 1 line
Quoted 1 line
huge L3-cache (few MBs). When the VCPU starts running some of
the
Quoted 3 lines
Quoted 3 lines
contents guest RAM will be still in L3-cache and VCPU runs with MMU off (i.e. cacheing off) hence VCPU will bypass L3-cache and see incorrect contents. To solve this problem we need to flush
the
Quoted 1 line
Quoted 1 line
guest RAM contents before they are accessed by first time by
VCPU.
Quoted 7 lines
Quoted 1 line
ok, I'm with you that far.
But is it also not true that we need to decide between:
A.1: Flush the entire guest RAM before running the VCPU A.2: Flush the pages as we fault them in
Yes, thats the decision we have to make.
Quoted 1 line
Quoted 6 lines
Quoted 4 lines
And (independently):
B.1: Use
__flush_dcache_range
B.2: Use something else + outer cache framework for arm64This would be
__flush_dcache_all()+ outer cache flush all.We need to be careful here since the
__flush_dcache_all()operation uses
cache maintenance by set/way and these are *local* to a CPU (IOW not broadcast). Do you have any guarantee that dirty cache lines don't migrate between CPUs and__flush_dcache_all()wouldn't miss them? Architecturally we don't, so this is not a safe operation that would guarantee L1 cache flushing (we probably need to revisit some of the__flush_dcache_all()calls in KVM, I haven't looked into this).Quoted 3 lines
So I think we are left to the range operation where the DC ops to PoC
would be enough for your L3.If
__flush_dcache_all()is *local" to a CPU then I guess DC ops to PoC
by range would be the only option.Quoted 3 lines
An outer cache flush all is probably only needed for cpuidle/suspend (the booting part should be handled by the boot loader).
Quoted 6 lines
Yes, cpuidle/suspend would definitely require outer cache maintenance.
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.
What about the I and C bits in SCTLR_EL1? Does L3 also honour these bits?
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 7 lines · Marc Zyngier
Quoted 3 lines · Anup Patel
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.What about the I and C bits in
SCTLR_EL1? Does L3 also honour these bits?
I would think so, it probably cares about how the transactions are presented at the bus level by the CPU.
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 11 lines · Catalin Marinas
Quoted 7 lines · Marc Zyngier
Quoted 3 lines · Anup Patel
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.What about the I and C bits in
SCTLR_EL1? Does L3 also honour these bits?I would think so, it probably cares about how the transactions are presented at the bus level by the CPU.
That'd make sense indeed. So we need to track both SCTLR_EL1.{I,C,M} in order to find out whether or not we need to clean the cache.
Mumble mumble...
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 7 lines · Marc Zyngier
Quoted 6 lines · Catalin Marinas
Quoted 5 lines · Marc Zyngier
Quoted 3 lines · Anup Patel
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
What about the I and C bits in
SCTLR_EL1? Does L3 also honour these bits?I would think so, it probably cares about how the transactions are presented at the bus level by the CPU.
That'd make sense indeed. So we need to track both SCTLR_EL1.{I,C,M} in order to find out whether or not we need to clean the cache.
<horrible hack>
Could you use a dummy ATS_* operation, then read the cacheability attributes out of the PAR?
</horrible hack>
Will
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Hi Will,
Quoted 14 lines · Will Deacon
Quoted 13 lines · Marc Zyngier
Quoted 11 lines · Catalin Marinas
Quoted 7 lines · Marc Zyngier
Quoted 3 lines · Anup Patel
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.What about the I and C bits in
SCTLR_EL1? Does L3 also honour these bits?I would think so, it probably cares about how the transactions are presented at the bus level by the CPU.
Quoted 7 lines
Quoted 2 lines
That'd make sense indeed. So we need to track both SCTLR_EL1.{I,C,M} in order to find out whether or not we need to clean the cache.
<horrible hack>
Could you use a dummy ATS_* operation, then read the cacheability attributes out of the PAR?
</horrible hack>
We will have to use ATS12xxx operation which can only be executed from EL2 or EL3. The host kernel runs at EL1 hence we will need to use HVC call to execute ATS12xxx operation in EL2 mode which in-turn means a world-switch overhead just to execute ATS12xxx operation.
Quoted 2 lines
Will
--Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Will DeaconHi Will,
Quoted 19 lines · Will Deacon
Quoted 9 lines · Marc Zyngier
Quoted 7 lines · Catalin Marinas
Quoted 6 lines · Marc Zyngier
Quoted 3 lines · Anup Patel
For KVM, we can avoid flushing d-cache to PoC every time in
coherent_icache_guest_page()by only doing it when Guest MMU is turned-off. This may reduce the performance penalty.What about the I and C bits in
SCTLR_EL1? Does L3 also honourthese
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
bits?
I would think so, it probably cares about how the transactions are presented at the bus level by the CPU.
That'd make sense indeed. So we need to track both SCTLR_EL1.{I,C,M} in
order to find out whether or not we need to clean the cache.
Quoted 5 lines
<horrible hack>
Could you use a dummy ATS_* operation, then read the cacheability attributes
out of the PAR?
</horrible hack>
That'd be doable, and maybe not that expensive (using ATS1, the Stage-1 translation is probably still cached after the translation fault). Actually, I suspect this is more correct than just looking at SCTLR_EL1, as it doesn't assume that the guest uses always cacheable memory when enables the caches.
Shame HPFAR_EL2 doesn't report this information though...
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelHi Anup,
Jumping late into this thread (holidays).
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and __flush_dcache_all() implementation does this dynamically).
For the I-cache all operation, that's correct, it only invalidates to the PoU but it doesn't need to go any further, you do the D-cache maintenance for this (no point in duplicating functionality).
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.
Do you mean the CPU is not aware of the L3 cache? Does CLIDR_EL1 report an L3 cache on your implementation?
It's not clear to me whether your L3 cache is inner or outer (or a mix). You say that D-cache maintenance to PoC flushes the L3 which looks to me like an inner cache, in which cache it should be reported in the LoC bits in CLIDR_EL1.
Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPU
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPU
Do you have specific instructions for flushing the L3 cache only? It's not clear to me what an outer-cache framework would to on AArch64. It was added on AArch32 for the L2x0/PL310 which need separate instructions by physical address for flushing the cache. I really hope we don't get these again on ARMv8 hardware.
Quoted 4 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.
flush_icache_range() is meant to flush only to the PoU to ensure the I-D cache coherency. I don't think we should change this.
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 11 lines · Catalin Marinas
Hi Anup,
Jumping late into this thread (holidays).
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 6 lines
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and
__flush_dcache_all()implementation does this dynamically).
The L3-cache is not visible to CPU. It is totally independent and transparent to CPU.
Quoted 5 lines
For the I-cache all operation, that's correct, it only invalidates to the PoU but it doesn't need to go any further, you do the D-cache maintenance for this (no point in duplicating functionality).
Quoted 7 lines
Quoted 4 lines
Also, Guest Linux already does
__flush_dcache_all()from__cpu_setup()at bootup time which does not work for us when L3-cache is enabled so, there is no point is adding one more__flush_dcache_all()upon first run of VCPU in KVM ARM64.Do you mean the CPU is not aware of the L3 cache? Does
CLIDR_EL1report an L3 cache on your implementation?
Yes, CPU is not aware of L3-cache and its state.
In our case, CLIDR_EL1 does not report L3-cache.
Quoted 18 lines
It's not clear to me whether your L3 cache is inner or outer (or a mix). You say that D-cache maintenance to PoC flushes the L3 which looks to me like an inner cache, in which cache it should be reported in the LoC bits in
CLIDR_EL1.Quoted 5 lines
IMHO, we are left with following options:
1. Flush all RAM regions of VCPU using__flush_dcache_range()upon first run of VCPU
2. Implement outer-cache framework for ARM64 and flush all caches + outer cache (i.e. L3-cache) upon first run of VCPUDo you have specific instructions for flushing the L3 cache only? It's not clear to me what an outer-cache framework would to on AArch64. It was added on AArch32 for the L2x0/PL310 which need separate instructions by physical address for flushing the cache. I really hope we don't get these again on ARMv8 hardware.
Quoted 7 lines
Quoted 4 lines
3. Use an alternate version of
flush_icache_range()which will flush d-cache by PoC instead of PoU. We can also ensure thatcoherent_icache_guest_page()function will be called upon Stage2 prefetch aborts only.
flush_icache_range()is meant to flush only to the PoU to ensure the I-D cache coherency. I don't think we should change this.
We did not mean to change flush_icache_range() instead we suggest to have separate flush_icache_range_kvm() for KVM ARM64 which flushes d-cache to PoC.
Quoted 3 lines
--
Catalin
--Anup
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 16 lines · Anup Patel
Quoted 7 lines · Catalin Marinas
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 6 lines
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and
__flush_dcache_all()implementation does this dynamically).
Quoted 2 lines
The L3-cache is not visible to CPU. It is totally independent and transparent to CPU.
OK. But you say that operations like DC CIVAC actually flush the L3? So I don't see it as completely transparent to the CPU.
Do you have any configuration bits which would make the L3 completely transparent like always caching even when accesses are non-cacheable and DC ops to PoC ignoring it?
Now, back to the idea of outer_cache framework for arm64. Does your CPU have separate instructions for flushing this L3 cache?
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 17 lines · Catalin Marinas
Quoted 16 lines · Anup Patel
Quoted 13 lines · Catalin Marinas
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and
__flush_dcache_all()implementation does this dynamically).
Quoted 5 lines
Quoted 2 lines
The L3-cache is not visible to CPU. It is totally independent and transparent to CPU.
OK. But you say that operations like DC CIVAC actually flush the L3? So I don't see it as completely transparent to the CPU.
It is transparent from CPU perspective. In other words, there is nothing in CPU for controlling/monitoring L3-cache.
Quoted 4 lines
Do you have any configuration bits which would make the L3 completely transparent like always caching even when accesses are non-cacheable and DC ops to PoC ignoring it?
Actually, L3-cache monitors the types of read/write generated by CPU (i.e. whether the request is cacheable/non-cacheable or whether the request is due to DC ops to PoC, or ...).
To answer your query, there is no configuration to have L3 caching when accesses are non-cacheable and DC ops to PoC.
Quoted 3 lines
Now, back to the idea of
outer_cacheframework for arm64. Does your CPU have separate instructions for flushing this L3 cache?
No, CPU does not have separate instruction for flushing L3-cache. On the other hand, L3-cache has MMIO registers which can be use to explicitly flush L3-cache.
Quoted 3 lines
--
Catalin
--Anup
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 19 lines · Anup Patel
Quoted 17 lines · Catalin Marinas
Quoted 16 lines · Anup Patel
Quoted 13 lines · Catalin Marinas
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and
__flush_dcache_all()implementation does this dynamically).
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
The L3-cache is not visible to CPU. It is totally independent and transparent to CPU.
OK. But you say that operations like DC CIVAC actually flush the L3? So I don't see it as completely transparent to the CPU.
It is transparent from CPU perspective. In other words, there is nothing in CPU for controlling/monitoring L3-cache.
We probably have a different understanding of "transparent". It doesn't look to me like any more transparent than the L1 or L2 cache. Basically, from a software perspective, it needs maintenance. Whether the CPU explicitly asks the L3 cache for this or the L3 cache figures it on its own based on the L1/L2 operations is irrelevant.
It would have been transparent if the software didn't need to know about it at all, but it's not the case.
Quoted 10 lines
Quoted 3 lines
Do you have any configuration bits which would make the L3 completely transparent like always caching even when accesses are non-cacheable and DC ops to PoC ignoring it?
Actually, L3-cache monitors the types of read/write generated by CPU (i.e. whether the request is cacheable/non-cacheable or whether the request is due to DC ops to PoC, or ...).
To answer your query, there is no configuration to have L3 caching when accesses are non-cacheable and DC ops to PoC.
So it's an outer cache with some "improvements" to handle DC ops to PoC. I think it was a pretty bad decision on the hardware side as we really try to get rid of outer caches for many reasons:
1. Non-standard cache flushing operations (MMIO-based)
2. It may require cache maintenance by physical address - something hard to get in a guest OS (unless you virtualise L3 cache maintenance)
3. Are barriers like DSB propagated correctly? Does a DC op to PoC followed by DSB ensure that the L3 drained the cachelines to RAM?
I think point 2 isn't required because your L3 detects DC ops to PoC. I hope point 3 is handled correctly (otherwise look how "nice" the mb() macro on arm is to cope with L2x0).
If only 1 is left, we don't need the full outer_cache framework but it still needs to be addressed since the assumption is that flush_cache_all (or __flush_dcache_all) flushes all cache levels. These are not used in generic code but are used during kernel booting, KVM and cpuidle drivers.
Quoted 6 lines
Quoted 2 lines
Now, back to the idea of
outer_cacheframework for arm64. Does your CPU have separate instructions for flushing this L3 cache?No, CPU does not have separate instruction for flushing L3-cache. On the other hand, L3-cache has MMIO registers which can be use to explicitly flush L3-cache.
I guess you use those in your firmware or boot loader since Linux requires clean/invalidated caches at boot (and I plan to push a patch which removes kernel D-cache cleaning during boot to spot such problems early). A cpuidle driver would probably need this as well.
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 26 lines · Catalin Marinas
Quoted 25 lines · Anup Patel
Quoted 22 lines · Catalin Marinas
Quoted 16 lines · Anup Patel
Quoted 7 lines · Catalin Marinas
Quoted 5 lines · Anup Patel
The approach of flushing d-cache by set/way upon first run of VCPU will not work because for set/way operations ARM ARM says: "For set/way operations, and for All (entire cache) operations, the point is defined to be to the next level of caching". In other words, set/way operations work upto point of unification.
Quoted 6 lines
I don't understand where you got the idea that set/way operations work up to the point of unification. This is incorrect, the set/way operations work on the level of cache specified by bits 3:1 in the register passed to the DC CISW instruction. For your L3 cache, those bits would be 2 (and
__flush_dcache_all()implementation does this dynamically).Quoted 2 lines
The L3-cache is not visible to CPU. It is totally independent and transparent to CPU.
OK. But you say that operations like DC CIVAC actually flush the L3? So I don't see it as completely transparent to the CPU.
Quoted 8 lines
Quoted 2 lines
It is transparent from CPU perspective. In other words, there is nothing in CPU for controlling/monitoring L3-cache.
We probably have a different understanding of "transparent". It doesn't look to me like any more transparent than the L1 or L2 cache. Basically, from a software perspective, it needs maintenance. Whether the CPU explicitly asks the L3 cache for this or the L3 cache figures it on its own based on the L1/L2 operations is irrelevant.
Yes, we have a different understanding regarding "transparent". The goal behind our L3 is to have external cache such that CPU is not aware of it and CPU would work fine even if L3 is turned-off on-the-fly. Further such L3 has to be smarter than normal outer-caches because it will be maintained automatically by observing CPU operations.
Quoted 3 lines
It would have been transparent if the software didn't need to know about it at all, but it's not the case.
Currently, KVM is the only place where we need L3 maintenance because Guest starts with MMU turned-off otherwise we don't need L3 maintenance in kernel for any kind of IO or Subsystem.
Quoted 9 lines
Quoted 9 lines
Quoted 4 lines
Do you have any configuration bits which would make the L3 completely transparent like always caching even when accesses are non-cacheable and DC ops to PoC ignoring it?
Actually, L3-cache monitors the types of read/write generated by CPU (i.e. whether the request is cacheable/non-cacheable or whether the request is due to DC ops to PoC, or ...).
Quoted 6 lines
Quoted 2 lines
To answer your query, there is no configuration to have L3 caching when accesses are non-cacheable and DC ops to PoC.
So it's an outer cache with some "improvements" to handle DC ops to PoC. I think it was a pretty bad decision on the hardware side as we really try to get rid of outer caches for many reasons:
Getting rid off outer-caches (such as in this context) may make sense for Embedded/Low-end systems but for Servers L3-cache makes lot of sense.
Claiming this to be a bad decision all depends on what end application you are looking at.
Quoted 5 lines
1. Non-standard cache flushing operations (MMIO-based)
2. It may require cache maintenance by physical address - something hard to get in a guest OS (unless you virtualise L3 cache maintenance)
We don't have cache maintenance by physical address in our L3-cache.
Quoted 2 lines
3. Are barriers like DSB propagated correctly? Does a DC op to PoC followed by DSB ensure that the L3 drained the cachelines to RAM?
Yes, DSB works perfectly fine for us with L3. Yes, DC op to PoC followed by DSB works fine with L3 too.
Quoted 14 lines
I think point 2 isn't required because your L3 detects DC ops to PoC. I hope point 3 is handled correctly (otherwise look how "nice" the
mb()macro on arm is to cope with L2x0).If only 1 is left, we don't need the full
outer_cacheframework but it still needs to be addressed since the assumption is thatflush_cache_all(or__flush_dcache_all) flushes all cache levels. These are not used in generic code but are used during kernel booting, KVM and cpuidle drivers.Quoted 3 lines
Quoted 2 lines
Now, back to the idea of
outer_cacheframework for arm64. Does your CPU have separate instructions for flushing this L3 cache?
Quoted 8 lines
Quoted 3 lines
No, CPU does not have separate instruction for flushing L3-cache. On the other hand, L3-cache has MMIO registers which can be use to explicitly flush L3-cache.
I guess you use those in your firmware or boot loader since Linux requires clean/invalidated caches at boot (and I plan to push a patch which removes kernel D-cache cleaning during boot to spot such problems early). A cpuidle driver would probably need this as well.
Yes, cpuidle would be another place where we may need L3-cache maintenance. In other words, if we need to power-off L3-cache from kernel then we need to first flush it.
Quoted 3 lines
--
Catalin
--Anup
Catalin Marinas
maintainerPATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 14 lines · Anup Patel
Quoted 11 lines · Catalin Marinas
Quoted 6 lines · Anup Patel
Actually, L3-cache monitors the types of read/write generated by CPU (i.e. whether the request is cacheable/non-cacheable or whether the request is due to DC ops to PoC, or ...).
To answer your query, there is no configuration to have L3 caching when accesses are non-cacheable and DC ops to PoC.
So it's an outer cache with some "improvements" to handle DC ops to PoC. I think it was a pretty bad decision on the hardware side as we really try to get rid of outer caches for many reasons:
Quoted 5 lines
Getting rid off outer-caches (such as in this context) may make sense for Embedded/Low-end systems but for Servers L3-cache makes lot of sense.
Claiming this to be a bad decision all depends on what end application you are looking at.
It's not a bad decision to have big L3 cache, that's a very *good* decision for server applications. The bad part is that it is not fully integrated with the CPU (like allowing set/way operations to flush this cache level).
Quoted 6 lines
Quoted 4 lines
1. Non-standard cache flushing operations (MMIO-based)
2. It may require cache maintenance by physical address - something hard to get in a guest OS (unless you virtualise L3 cache maintenance)We don't have cache maintenance by physical address in our L3-cache.
Great.
Quoted 5 lines
Quoted 2 lines
3. Are barriers like DSB propagated correctly? Does a DC op to PoC followed by DSB ensure that the L3 drained the cachelines to RAM?
Yes, DSB works perfectly fine for us with L3. Yes, DC op to PoC followed by DSB works fine with L3 too.
Even better.
See my other comment on flush_dcache_all() use in the kernel/KVM and why I don't think it is suitable (which leaves us with DC ops to PoC in KVM).
Quoted 17 lines
Quoted 17 lines
I think point 2 isn't required because your L3 detects DC ops to PoC. I hope point 3 is handled correctly (otherwise look how "nice" the
mb()macro on arm is to cope with L2x0).If only 1 is left, we don't need the full
outer_cacheframework but it still needs to be addressed since the assumption is thatflush_cache_all(or__flush_dcache_all) flushes all cache levels. These are not used in generic code but are used during kernel booting, KVM and cpuidle drivers.Quoted 6 lines
Quoted 2 lines
Now, back to the idea of
outer_cacheframework for arm64. Does your CPU have separate instructions for flushing this L3 cache?No, CPU does not have separate instruction for flushing L3-cache. On the other hand, L3-cache has MMIO registers which can be use to explicitly flush L3-cache.
Quoted 8 lines
Quoted 4 lines
I guess you use those in your firmware or boot loader since Linux requires clean/invalidated caches at boot (and I plan to push a patch which removes kernel D-cache cleaning during boot to spot such problems early). A cpuidle driver would probably need this as well.
Yes, cpuidle would be another place where we may need L3-cache maintenance. In other words, if we need to power-off L3-cache from kernel then we need to first flush it.
The problem is if you need to disable the MMU in the kernel, you would need to flush the L3 cache first. Normally this would be done in the firmware with PSCI but most likely not the case for the Applied hardware.
Signature · 1 line
Catalin
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Anup PatelQuoted 233 lines · Anup Patel
Quoted 221 lines · Christoffer Dall
Quoted 220 lines · Marc Zyngier
Quoted 206 lines · Anup Patel
Hi Marc,
Quoted 184 lines · Marc Zyngier
Quoted 169 lines · Anup Patel
Quoted 155 lines · Marc Zyngier
Quoted 135 lines · Anup Patel
Hi Marc,
Quoted 116 lines · Marc Zyngier
Hi Anup,
Quoted 92 lines · Anup Patel
Quoted 59 lines · Marc Zyngier
Hi Pranav,
Quoted 34 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty
content belonging to the guest page in L3-cache. To tackle this,
we need to flush such dirty content from d-cache so that guest
will see correct contents of guest page when guest MMU is disabled.The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
Quoted 7 lines
+ __flush_dcache_area((void *) hva, PAGE_SIZE); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all();[adding Will to the discussion as we talked about this in the past]
That's definitely an issue, but I'm not sure the fix is to hit the
data
cache on each page mapping. It looks overkill.Quoted 6 lines
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools
knows which bits of the guest memory have been touched, and can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 6 lines
The alternative is do it in the kernel before running any vcpu
- but
that's not very nice either (have to clean the whole of the guest
memory, which makes a full dcache clean more appealing).Quoted 8 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simple hence we went for this.If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that it fixes your
issue.Quoted 4 lines
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 4 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
Quoted 8 lines
Quoted 2 lines
IMHO, this patch enforces incorrect memory attribute for Guest/VM when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.Quoted 7 lines
You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:
https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.htmlQuoted 8 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).Quoted 9 lines
A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out of
curiosity: is that a made up example or something you actually have?Quoted 9 lines
Quoted 4 lines
Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Quoted 5 lines
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
Read my question again: The bootloaders running with the MMU off in a VM will only disable the MMU for Stage-1 translations. Stage-2 translations are still enforced using hte Stage-2 page tables and their attributes for all mappings to devices will still enforce strongly-ordered device type memory.
Again, what does it matter if the device is DMA-capable or not?
-Christoffer
A message that is not in the archive (sent off-list or not yet received): CALrVBkvEP1Q0mKpv8ViOTLRvW2ks18MQXgmurSBHn+aJcz+=gw@mail.gmail.com
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 249 lines · Anup Patel
Quoted 247 lines · Christoffer Dall
Quoted 89 lines · Anup Patel
Quoted 87 lines · Christoffer Dall
Quoted 86 lines · Marc Zyngier
Quoted 85 lines · Anup Patel
Hi Marc,
On Thu, Aug 15, 2013 at 8:17 PM, Marc Zyngier <[email protected]
Quoted 80 lines · wrote:
Quoted 79 lines · Anup Patel
Quoted 75 lines · Marc Zyngier
Quoted 73 lines · Anup Patel
Hi Marc,
Quoted 66 lines · Marc Zyngier
Hi Anup,
Quoted 60 lines · Anup Patel
Quoted 54 lines · Marc Zyngier
Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
That's definitely an issue, but I'm not sure the fix is to
hit
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 10 lines
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space. I am
sure
other architectures don't have such cache cleaning in user-space for
KVM.Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
The alternative is do it in the kernel before running any
vcpu
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 4 lines
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of VCPU was
our second option but current approach seemed very simplehence
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
we went for this.
If more people vote for full d-cache clean upon first run of VCPU then
we should revise this patch.Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that itfixes
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 15 lines
Quoted 7 lines
your
issue.As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
IMHO, this patch enforces incorrect memory attribute for
Guest/VM
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 7 lines
Quoted 1 line
when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.html
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 28 lines
Quoted 9 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 1 line
Quoted 1 line
Quoted 1 line
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out
of
Quoted 17 lines
Quoted 11 lines
Quoted 6 lines
curiosity: is that a made up example or something you actually have?
Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
Quoted 10 lines
Quoted 6 lines
Read my question again: The bootloaders running with the MMU off in a VM will only disable the MMU for Stage-1 translations. Stage-2 translations are still enforced using hte Stage-2 page tables and their attributes for all mappings to devices will still enforce strongly-ordered device type memory.
Please read my reply again. Also try to read-up SATA AHCI spec.
Can you please explain how the specs realate to my question?
The only case I can see is if the guest puts data in a DMA region that it expects the device to read. Is this the case?
Just so we're clear, this is quite different from programming the device through the device memory.
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 261 lines · Christoffer Dall
Quoted 249 lines · Anup Patel
Quoted 247 lines · Christoffer Dall
Quoted 89 lines · Anup Patel
Quoted 87 lines · Christoffer Dall
Quoted 86 lines · Marc Zyngier
Quoted 85 lines · Anup Patel
Hi Marc,
On Thu, Aug 15, 2013 at 8:17 PM, Marc Zyngier <[email protected]
Quoted 80 lines · wrote:
Quoted 79 lines · Anup Patel
Quoted 75 lines · Marc Zyngier
Quoted 73 lines · Anup Patel
Hi Marc,
Quoted 66 lines · Marc Zyngier
Hi Anup,
Quoted 60 lines · Anup Patel
On Wed, Aug 14, 2013 at 5:34 PM, Marc Zyngier
<[email protected]>
wrote:Hi Pranav,
On 2013-08-14 12:47, Pranavkumar Sawargaonkar wrote:
Systems with large external L3-cache (few MBs), might have
dirty
content belonging to the guest page in L3-cache. To tackle
this,
we need to flush such dirty content from d-cache so that
guest
will see correct contents of guest page when guest MMU is
disabled.The patch fixes coherent_icache_guest_page() for external
L3-cache.Signed-off-by: Pranavkumar Sawargaonkar
<[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
arch/arm64/include/asm/kvm_mmu.h | 2 ++
1 file changed, 2 insertions(+)diff --git a/arch/arm64/include/asm/kvm_mmu.h
b/arch/arm64/include/asm/kvm_mmu.h
index efe609c..5129038 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -123,6 +123,8 @@ static inline void
coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)
if (!icache_is_aliasing()) { /* PIPT */
unsigned long hva = gfn_to_hva(kvm, gfn);
flush_icache_range(hva, hva + PAGE_SIZE);
+ /* Flush d-cache for systems with external
caches. */
+ __flush_dcache_area((void *) hva, PAGE_SIZE);
} else if (!icache_is_aivivt()) { /* non
ASID-tagged
VIVT
*/
/* any kind of VIPT cache */
__flush_icache_all();[adding Will to the discussion as we talked about this in the
past]Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
That's definitely an issue, but I'm not sure the fix is to
hit
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
Quoted 22 lines
the
data
cache on each page mapping. It looks overkill.Wouldn't it be enough to let userspace do the cache cleaning?
kvmtools
knows which bits of the guest memory have been touched, and
can do a
"DC
DVAC" on this region.It seems a bit unnatural to have cache cleaning is user-space.
I am
sure
other architectures don't have such cache cleaning in
user-space for
KVM.Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
The alternative is do it in the kernel before running any
vcpu
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines
- but
that's not very nice either (have to clean the whole of the
guest
memory, which makes a full dcache clean more appealing).Actually, cleaning full d-cache by set/way upon first run of
VCPU was
our second option but current approach seemed very simplehence
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 5 lines
we went for this.
If more people vote for full d-cache clean upon first run of
VCPU then
we should revise this patch.Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Can you please give the attached patch a spin on your HW? I've boot-tested
it on a model, but of course I can't really verify that itfixes
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 15 lines
Quoted 7 lines
your
issue.As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
HCR_EL2.DC=1 means all memory access for Stage1 MMU off are treated as "Normal Non-shareable, Inner Write-Back Write-Allocate,
Outer Write-Back Write-Allocate memory"Quoted 6 lines
HCR_EL2.DC=0 means all memory access for Stage1 MMU off are treated as "Strongly-ordered device memory"
Now if Guest/VM access hardware MMIO devices directly (such as VGIC CPU interface) with MMU off then MMIO devices will be accessed as normal memory when HCR_EL2.DC=1.
I don't think so. Stage-2 still applies, and should force MMIO to be
accessed as device memory.Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
The HCR_EL2.DC=1 makes sense only if we have all software emulated devices for Guest/VM which is not true for KVM ARM or KVM ARM64 because we use VGIC.
IMHO, this patch enforces incorrect memory attribute for
Guest/VM
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 7 lines
Quoted 1 line
when Stage1 MMU is off.
See above. My understanding is that HCR.DC controls the default output of
Stage-1, and Stage-2 overrides still apply.You had mentioned that
PAGE_S2_DEVICEattribute was redundant and wanted guest to decide the memory attribute. In other words, you
did not want to enforce any memory attribute in Stage2.Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Please refer to this patch
https://patchwork.kernel.org/patch/2543201/This patch has never been merged. If you carry on following the discussion,
you will certainly notice it was dropped for a very good reason:https://lists.cs.columbia.edu/pipermail/kvmarm/2013-May/005827.html
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 28 lines
Quoted 9 lines
So Stage-2 memory attributes are used, they are not going away, and they are
essential to the patch I sent this morning.M.--
Fast, cheap, reliable. Pick two.HCR_EL2.DC=1 will break Guest/VM bootloader/firmware if the Guest/VM is
provided a DMA-capable device in pass-through mode. The reason being bootloader/firmware typically don't enable MMU and such bootloader/firmware
will programme a pass-through DMA-capable device without any flushes to
guest RAM (because it has not enabled MMU).A good example of such a device would be SATA AHCI controller given to a
Guest/VM as direct access (using SystemMMU) and Guest bootloader/firmware
accessing this SATA AHCI controller to load kernel images from SATA disk.
In this situation, we will have to hack Guest bootloader/firmware AHCI driver to
explicitly flushes to Guest RAM (because have HCR_EL2.DC=1).Quoted 1 line
Quoted 1 line
Quoted 1 line
OK. So *that* looks like a valid argument against HCR_EL2.DC==1. Out
of
Quoted 17 lines
Quoted 11 lines
Quoted 6 lines
curiosity: is that a made up example or something you actually have?
Back to square one:
Can you please benchmark the various cache cleaning options (global at startup time, per-page on S2 translation fault, and user-space)?Eh, why is this a more valid argument than the vgic? The device passthrough Stage-2 mappings would still have the Stage-2 memory attributes to configure that memory region as device memory. Why is it relevant if the device is DMA-capable in this context?
Most ARM bootloader/firmware run with MMU off hence, they will not do explicit cache flushes when programming DMA-capable device. Now If HCR_EL2.DC=1 then Guest RAM updates done by bootloader/firmware will not be visible to DMA-capable device.
Quoted 10 lines
Quoted 6 lines
Read my question again: The bootloaders running with the MMU off in a VM will only disable the MMU for Stage-1 translations. Stage-2 translations are still enforced using hte Stage-2 page tables and their attributes for all mappings to devices will still enforce strongly-ordered device type memory.
Please read my reply again. Also try to read-up SATA AHCI spec.
Quoted 8 lines
Can you please explain how the specs realate to my question?
The only case I can see is if the guest puts data in a DMA region that it expects the device to read. Is this the case?
Just so we're clear, this is quite different from programming the device through the device memory.
ok, I see from the specs. It's not just DMA-capable, but actually DMA-programmable. In that case setting DC=1 would be problematic.
Thanks for being so helpful in explaining this ;)
-Christoffer
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Marc ZyngierHi Marc,
Quoted 79 lines · Marc Zyngier
Hi Anup,
Quoted 57 lines · Anup Patel
Quoted 37 lines · Marc Zyngier
Hi Pranav,
Quoted 28 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) if (!icache_is_aliasing()) { /* PIPT */unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); } else if (!icache_is_aivivt()) { /* non ASID-tagged VIVT *//* any kind of VIPT cache */ Quoted 1 line
__flush_icache_all();[adding Will to the discussion as we talked about this in the past]
Quoted 6 lines
That's definitely an issue, but I'm not sure the fix is to hit the data cache on each page mapping. It looks overkill.
Wouldn't it be enough to let userspace do the cache cleaning? kvmtools knows which bits of the guest memory have been touched, and can do a "DC DVAC" on this region.
It seems a bit unnatural to have cache cleaning is user-space. I am sure other architectures don't have such cache cleaning in user-space for KVM.
Quoted 4 lines
The alternative is do it in the kernel before running any vcpu - but that's not very nice either (have to clean the whole of the guest memory, which makes a full dcache clean more appealing).
Quoted 6 lines
Actually, cleaning full d-cache by set/way upon first run of VCPU was our second option but current approach seemed very simple hence we went for this.
If more people vote for full d-cache clean upon first run of VCPU then we should revise this patch.
Can you please give the attached patch a spin on your HW? I've boot-tested it on a model, but of course I can't really verify that it fixes your issue.
As far as I can see, it should fix it without any additional flushing.
Please let me know how it goes.
Thanks,
Quoted 8 lines
M.--
Fast, cheap, reliable. Pick two._______________________________________________
kvmarm mailing list
kvmarm at lists.cs.columbia.edu
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
Surely we will try this patch.
Just to add few more points to this discussion :
Actually we are already flushing d-cache in "coherent_icache_guest_page" by calling flush_icache_range.
Now in my patch i am doing same thing one more time by calling __flush_dcache_area just below flush_icache_range.
Main difference between d-cache flushing in above two routines is :flush_icache_range is flushing d-cache by point of unification (dc cvau) and
__flush_dcache_area is flushing that by point of coherency (dc civac).
Now since second routine is doing it by coherency it is making L3C to be coherent and sync changes with immediate effect and solving the issue.
Thanks,
Pranav
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Hi Pranav,
On 2013-08-15 09:39, Pranavkumar Sawargaonkar wrote:
[...]
Quoted 15 lines
Just to add few more points to this discussion :
Actually we are already flushing d-cache in
"coherent_icache_guest_page"
by callingflush_icache_range.Now in my patch i am doing same thing one more time by calling
__flush_dcache_areajust belowflush_icache_range.Main difference between d-cache flushing in above two routines is :
flush_icache_rangeis flushing d-cache by point of unification (dc cvau) and
__flush_dcache_areais flushing that by point of coherency (dc
civac).
Quoted 3 lines
Now since second routine is doing it by coherency it is making L3C to be coherent and sync changes with immediate effect and solving the issue.
I understand that. What I object to is that always cleaning to PoC is expensive (both in terms of latency and energy), and doing so for each page, long after the MMU has been enabled feels like a great loss of performance.
My gut feeling is that a single clean operation at startup time (whether it is from the kernel or userspace) would be a lot better.
Thanks,
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Pranavkumar SawargaonkarQuoted 7 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.
I don't understand KVM but the commit message caught my attention.
You are referring to clean the external/outer L3 cache but using internal cache maintenance APIs. You should be using outer_cache APIs if you really intend to do outer cache maintenance ?
Is the external/outer L3 cache not unified ?
Or did you mean to flush d-cache to the point of coherency(external L3 in your case)? If so it's not clear from the commit log.
Regards,
Sudeep
Quoted 20 lines
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)if (!icache_is_aliasing ()) {/* PIPT */ unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); }else if (!icache_is_aivivt ()) {/* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all ();
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
Quoted 11 lines · Sudeep KarkadaNagesha
Quoted 7 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.I don't understand KVM but the commit message caught my attention.
Quoted 7 lines
You are referring to clean the external/outer L3 cache but using internal cache maintenance APIs. You should be using
outer_cacheAPIs if you really intend to do outer cache maintenance ?
Is the external/outer L3 cache not unified ?Or did you mean to flush d-cache to the point of coherency(external L3 in your case)? If so it's not clear from the commit log.
Yes, we mean to flush d-cache to the point of coherency (which in our case is external L3-cache).
The patch description is more in-terms of KVM virtualization but, we can certainly add more details.
Quoted 23 lines
Regards,
SudeepQuoted 19 lines
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +2
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)if (!icache_is_aliasing ()) {/* PIPT */ unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */ +__flush_dcache_area ((void *) hva,PAGE_SIZE ); }else if (!icache_is_aivivt ()) {/* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all ();
Quoted 8 lines
Quoted 1 line
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Regards,
Anup
PATCH 1/1 ARM64: KVM: Fix coherent_icache_guest_page() for host with external L3-cache.
In reply to Pranavkumar SawargaonkarQuoted 18 lines · Pranavkumar Sawargaonkar
Systems with large external L3-cache (few MBs), might have dirty content belonging to the guest page in L3-cache. To tackle this, we need to flush such dirty content from d-cache so that guest will see correct contents of guest page when guest MMU is disabled.
The patch fixes
coherent_icache_guest_page()for external L3-cache.Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Anup Patel <[email protected]>--- arch/arm64/include/asm/kvm_mmu.h | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/include/asm/kvm_mmu.h +0
-0
arch/arm64/include/asm/kvm_mmu.h static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) · line 126
Quoted 4 lines
arch/arm64/include/asm/kvm_mmu.h +1
-0@@ -123,6 +123,8 @@ static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn)if (!icache_is_aliasing ()) {/* PIPT */ unsigned long hva =gfn_to_hva (kvm, gfn);flush_icache_range (hva, hva +PAGE_SIZE ); +/* Flush d-cache for systems with external caches. */
This comment is nowhere near explanatory enough for someone who comes by later and tries to figure out why we're flushing the dcache on every page we swap in under given circumstances. Yes, you can do git blame, until you modify the line for some other reason and it just becomes a pain.
arch/arm64/include/asm/kvm_mmu.h static inline void coherent_icache_guest_page(struct kvm *kvm, gfn_t gfn) · line 127
Quoted 1 line
arch/arm64/include/asm/kvm_mmu.h +1
-0+__flush_dcache_area ((void *) hva,PAGE_SIZE );
eh, why is this only relevant for a non-aliasing icache? In fact, this does not seem to be icache related at all, but rather instruction-stream to dcache related, which would warrant either a rename of the function to something more generic or a separate function.
Quoted 10 lines
arch/arm64/include/asm/kvm_mmu.h +0
-0}else if (!icache_is_aivivt ()) {/* non ASID-tagged VIVT */ /* any kind of VIPT cache */ __flush_icache_all ();Signature · 6 lines
1.7.9.5 _______________________________________________ kvmarm mailing list kvmarm at lists.cs.columbia.edu https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm