intel_iommu: handle invalid ce for shadow sync
PATCH 1/1 intel_iommu: handle invalid ce for shadow sync
There are two callers for vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the caller vtd_sync_shadow_page_table() where we need to fetch the context entry.
Meanwhile, we should handle VTD_FR_CONTEXT_ENTRY_P properly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.
Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driver
The patch should fix it.
--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)
hw/i386/intel_iommu.c +33 -21
@@ -37,6 +37,8 @@#include "kvm_i386.h" #include "trace.h" +static void vtd_address_space_unmap (VTDAddressSpace *as,IOMMUNotifier *n); +static void vtd_define_quad (IntelIOMMUState *s,hwaddr addr,uint64_t val,uint64_t wmask,uint64_t w1cmask) {
@@ -1047,39 +1049,49 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, .notify_unmap = true, .aw = s->aw_bits, .as = vtd_as, + .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi), }; -VTDContextEntry ce_cache; + +return vtd_page_walk (ce, addr, addr + size, &info); +} + +static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) +{int ret; +VTDContextEntry ce; +IOMMUNotifier *n; -if (ce) { -/* If the caller provided context entry, use it */ - ce_cache = *ce; - }else { -/* If the caller didn't provide ce, try to fetch */ - ret =vtd_dev_to_context_entry (s,pci_bus_num (vtd_as->bus), - vtd_as->devfn, &ce_cache); -if (ret) { + ret =vtd_dev_to_context_entry (vtd_as->iommu_state, +pci_bus_num (vtd_as->bus), + vtd_as->devfn, &ce); +if (ret) { +if (ret == -VTD_FR_CONTEXT_ENTRY_P ) { +/* +* It's a valid scenario to have a context entry that is +* not present. For example, when a device is removed +* from an existing domain then the context entry will be +* zeroed by the guest before it was put into another +* domain. When this happens, instead of synchronizing +* the shadow pages we should invalidate all existing +* mappings and notify the backends. +*/ + IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) { +vtd_address_space_unmap (vtd_as, n); + } + }else {/* * This should not really happen, but in case it happens, * we just skip the sync for this time. After all we even * don't have the root table pointer! */ error_report_once ("%s: invalid context entry for bus 0x%x" -" devfn 0x%x" , - __func__,pci_bus_num (vtd_as->bus), - vtd_as->devfn); -return 0 ; +" devfn 0x%x" , __func__, +pci_bus_num (vtd_as->bus), vtd_as->devfn); } +return 0 ; } - info.domain_id =VTD_CONTEXT_ENTRY_DID (ce_cache.hi); - -return vtd_page_walk (&ce_cache, addr, addr + size, &info); -} - -static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) -{ -return vtd_sync_shadow_page_table_range (vtd_as,NULL ,0 ,UINT64_MAX ); +return vtd_sync_shadow_page_table_range (vtd_as, &ce,0 ,UINT64_MAX ); }/*
Signature · 1 line
2.17.1
Hi Peter,
Quoted 20 lines · Peter Xu
There are two callers for
vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the callervtd_sync_shadow_page_table()where we need to fetch the context entry.Meanwhile, we should handle
VTD_FR_CONTEXT_ENTRY_Pproperly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driverThe patch should fix it.
Quoted 9 lines
Reported-by Pei Zhang<[email protected]>Tested-by Pei Zhang<[email protected]>CC Pei Zhang<[email protected]>CC Alex Williamson<[email protected]>CC Jason Wang<[email protected]>CC Maxime Coquelin<[email protected]>CC Michael S. Tsirkin<[email protected]>CC QEMU Stable<[email protected]>
It seems like a regression as it wasn't reported earlier, isn't it? If it is, do you know what is the faulty commit?
Quoted 4 lines
Signed-off-by Peter Xu<[email protected]>--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)
Other than that, the patch looks good to me.
FWIW:
Thanks,
Maxime
Quoted 33 lines · Maxime Coquelin
Hi Peter,
Quoted 20 lines · Peter Xu
There are two callers for
vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the callervtd_sync_shadow_page_table()where we need to fetch the context entry.Meanwhile, we should handle
VTD_FR_CONTEXT_ENTRY_Pproperly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driverThe patch should fix it.
Quoted 9 lines
Reported-by Pei Zhang<[email protected]>Tested-by Pei Zhang<[email protected]>CC Pei Zhang<[email protected]>CC Alex Williamson<[email protected]>CC Jason Wang<[email protected]>CC Maxime Coquelin<[email protected]>CC Michael S. Tsirkin<[email protected]>CC QEMU Stable<[email protected]>
Quoted 2 lines
It seems like a regression as it wasn't reported earlier, isn't it? If it is, do you know what is the faulty commit?
I think it should be 63b88968f1 ("intel-iommu: rework the page walk logic", 2018-05-23). The old code would possibly unmap all (using the old replay logic) before we introduce the shadow page sync helpers and I must have overlooked on that point.
Quoted 9 lines
Quoted 5 lines
Signed-off-by Peter Xu<[email protected]>--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)Other than that, the patch looks good to me.
FWIW:Acked-by Maxime Coquelin <[email protected]>
Thanks,
Signature · 1 line
Peter Xu
Hi Peter,
Quoted 39 lines · Peter Xu
There are two callers for
vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the callervtd_sync_shadow_page_table()where we need to fetch the context entry.Meanwhile, we should handle
VTD_FR_CONTEXT_ENTRY_Pproperly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driverThe patch should fix it.
Reported-by Pei Zhang <[email protected]>Tested-by Pei Zhang <[email protected]>CC Pei Zhang <[email protected]>CC Alex Williamson <[email protected]>CC Jason Wang <[email protected]>CC Maxime Coquelin <[email protected]>CC Michael S. Tsirkin <[email protected]>CC QEMU Stable <[email protected]>Signed-off-by Peter Xu <[email protected]>--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)hw/i386/intel_iommu.c +0
-0
hw/i386/intel_iommu.c line 44
Quoted 8 lines
hw/i386/intel_iommu.c +2
-0@@ -37,6 +37,8 @@#include "kvm_i386.h" #include "trace.h" +static void vtd_address_space_unmap (VTDAddressSpace *as,IOMMUNotifier *n); +static void vtd_define_quad (IntelIOMMUState *s,hwaddr addr,uint64_t val,uint64_t wmask,uint64_t w1cmask) {
Comment above is outdated:
/* If context entry is NULL, we'll try to fetch it on our own. */
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1056
Quoted 10 lines
hw/i386/intel_iommu.c +4
-1@@ -1047,39 +1049,49 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, .notify_unmap = true, .aw = s->aw_bits, .as = vtd_as, + .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi), }; -VTDContextEntry ce_cache; + +return vtd_page_walk (ce, addr, addr + size, &info); +}
Maybe change would gain in clarity if split into 2 patches, code reorganization and fix on the side.
Thanks
Eric
Quoted 62 lines
hw/i386/intel_iommu.c +27
-20+ +static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) +{int ret; +VTDContextEntry ce; +IOMMUNotifier *n; -if (ce) { -/* If the caller provided context entry, use it */ - ce_cache = *ce; - }else { -/* If the caller didn't provide ce, try to fetch */ - ret =vtd_dev_to_context_entry (s,pci_bus_num (vtd_as->bus), - vtd_as->devfn, &ce_cache); -if (ret) { + ret =vtd_dev_to_context_entry (vtd_as->iommu_state, +pci_bus_num (vtd_as->bus), + vtd_as->devfn, &ce); +if (ret) { +if (ret == -VTD_FR_CONTEXT_ENTRY_P ) { +/* +* It's a valid scenario to have a context entry that is +* not present. For example, when a device is removed +* from an existing domain then the context entry will be +* zeroed by the guest before it was put into another +* domain. When this happens, instead of synchronizing +* the shadow pages we should invalidate all existing +* mappings and notify the backends. +*/ + IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) { +vtd_address_space_unmap (vtd_as, n); + } + }else {/* * This should not really happen, but in case it happens, * we just skip the sync for this time. After all we even * don't have the root table pointer! */ error_report_once ("%s: invalid context entry for bus 0x%x" -" devfn 0x%x" , - __func__,pci_bus_num (vtd_as->bus), - vtd_as->devfn); -return 0 ; +" devfn 0x%x" , __func__, +pci_bus_num (vtd_as->bus), vtd_as->devfn); } +return 0 ; } - info.domain_id =VTD_CONTEXT_ENTRY_DID (ce_cache.hi); - -return vtd_page_walk (&ce_cache, addr, addr + size, &info); -} - -static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) -{ -return vtd_sync_shadow_page_table_range (vtd_as,NULL ,0 ,UINT64_MAX ); +return vtd_sync_shadow_page_table_range (vtd_as, &ce,0 ,UINT64_MAX ); }/*
Quoted 44 lines · Auger Eric
Hi Peter,
Quoted 42 lines · Peter Xu
There are two callers for
vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the callervtd_sync_shadow_page_table()where we need to fetch the context entry.Meanwhile, we should handle
VTD_FR_CONTEXT_ENTRY_Pproperly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driverThe patch should fix it.
Reported-by Pei Zhang <[email protected]>Tested-by Pei Zhang <[email protected]>CC Pei Zhang <[email protected]>CC Alex Williamson <[email protected]>CC Jason Wang <[email protected]>CC Maxime Coquelin <[email protected]>CC Michael S. Tsirkin <[email protected]>CC QEMU Stable <[email protected]>Signed-off-by Peter Xu <[email protected]>--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)hw/i386/intel_iommu.c +0
-0@@ -37,6 +37,8 @@#include "kvm_i386.h" #include "trace.h"
Quoted 7 lines
hw/i386/intel_iommu.c line 44
Quoted 5 lines
hw/i386/intel_iommu.c +2
-0+static void vtd_address_space_unmap (VTDAddressSpace *as,IOMMUNotifier *n); +static void vtd_define_quad (IntelIOMMUState *s,hwaddr addr,uint64_t val,uint64_t wmask,uint64_t w1cmask) {Comment above is outdated:
/* If context entry is NULL, we'll try to fetch it on our own. */
Indeed.
Quoted 4 lines
Quoted 4 lines
hw/i386/intel_iommu.c +0
-0@@ -1047,39 +1049,49 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, .notify_unmap = true, .aw = s->aw_bits, .as = vtd_as,
Quoted 8 lines
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1056
Quoted 6 lines
hw/i386/intel_iommu.c +4
-1+ .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi), }; -VTDContextEntry ce_cache; + +return vtd_page_walk (ce, addr, addr + size, &info); +}Maybe change would gain in clarity if split into 2 patches, code reorganization and fix on the side.
Sure. Thanks for reviewing!
Signature · 1 line
Peter Xu
Quoted 30 lines · Peter Xu
There are two callers for
vtd_sync_shadow_page_table_range(), one provided a valid context entry and one not. Move that fetching operation into the callervtd_sync_shadow_page_table()where we need to fetch the context entry.Meanwhile, we should handle
VTD_FR_CONTEXT_ENTRY_Pproperly when synchronizing shadow page tables. Having invalid context entry there is perfectly valid when we move a device out of an existing domain. When that happens, instead of posting an error we invalidate the whole region.Without this patch, QEMU will crash if we do these steps:
(1) start QEMU with VT-d IOMMU and two 10G NICs (ixgbe)
(2) bind the NICs with vfio-pci in the guest
(3) start testpmd with the NICs applied
(4) stop testpmd
(5) rebind the NIC back to ixgbe kernel driverThe patch should fix it.
Reported-by Pei Zhang <[email protected]>Tested-by Pei Zhang <[email protected]>CC Pei Zhang <[email protected]>CC Alex Williamson <[email protected]>CC Jason Wang <[email protected]>CC Maxime Coquelin <[email protected]>CC Michael S. Tsirkin <[email protected]>CC QEMU Stable <[email protected]>Signed-off-by Peter Xu <[email protected]>
Quoted 3 lines
--- hw/i386/intel_iommu.c | 54 ++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 21 deletions(-)
Some nits, see below.
Quoted 52 lines
hw/i386/intel_iommu.c +24
-9@@ -37,6 +37,8 @@#include "kvm_i386.h" #include "trace.h" +static void vtd_address_space_unmap (VTDAddressSpace *as,IOMMUNotifier *n); +static void vtd_define_quad (IntelIOMMUState *s,hwaddr addr,uint64_t val,uint64_t wmask,uint64_t w1cmask) {@@ -1047,39 +1049,49 @@ static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, .notify_unmap = true, .aw = s->aw_bits, .as = vtd_as, + .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi), }; -VTDContextEntry ce_cache; + +return vtd_page_walk (ce, addr, addr + size, &info); +} + +static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) +{int ret; +VTDContextEntry ce; +IOMMUNotifier *n; -if (ce) { -/* If the caller provided context entry, use it */ - ce_cache = *ce; - }else { -/* If the caller didn't provide ce, try to fetch */ - ret =vtd_dev_to_context_entry (s,pci_bus_num (vtd_as->bus), - vtd_as->devfn, &ce_cache); -if (ret) { + ret =vtd_dev_to_context_entry (vtd_as->iommu_state, +pci_bus_num (vtd_as->bus), + vtd_as->devfn, &ce); +if (ret) { +if (ret == -VTD_FR_CONTEXT_ENTRY_P ) { +/* +* It's a valid scenario to have a context entry that is +* not present. For example, when a device is removed +* from an existing domain then the context entry will be +* zeroed by the guest before it was put into another +* domain. When this happens, instead of synchronizing +* the shadow pages we should invalidate all existing +* mappings and notify the backends.
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1086
Quoted 10 lines
hw/i386/intel_iommu.c +5
-0+*/ + IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) { +vtd_address_space_unmap (vtd_as, n); + } + }else {/* * This should not really happen, but in case it happens, * we just skip the sync for this time. After all we even * don't have the root table pointer! */
It looks to me the comment is not accurate, no root pointer is not the only reason for the failure of vtd_dev_to_context_entry().
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1071
Quoted 5 lines
hw/i386/intel_iommu.c +0
-4error_report_once("%s: invalid context entry for bus 0x%x" -" devfn 0x%x" , - __func__,pci_bus_num (vtd_as->bus), - vtd_as->devfn); -return 0 ;
I'm not quite sure error_report_once() is really needed here since all failures has been traced.
Quoted 6 lines
hw/i386/intel_iommu.c +3
-0+" devfn 0x%x" , __func__, + pci_bus_num(vtd_as->bus), vtd_as->devfn); } +return 0 ; }
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1095
Quoted 10 lines
hw/i386/intel_iommu.c +1
-8- info.domain_id =VTD_CONTEXT_ENTRY_DID (ce_cache.hi); - -return vtd_page_walk (&ce_cache, addr, addr + size, &info); -} - -static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) -{ -return vtd_sync_shadow_page_table_range (vtd_as,NULL ,0 ,UINT64_MAX ); +return vtd_sync_shadow_page_table_range (vtd_as, &ce,0 ,UINT64_MAX ); }
As has been discussed, this will left addr UINT64_MAX, it's better to have [start, end] instead of (start, range).
Thanks
Quoted 2 lines
hw/i386/intel_iommu.c +0
-0/*
On Mon, Oct 08, 2018 at 11:08:31AM +0800, Jason Wang wrote:
[...]
Quoted 40 lines
Quoted 26 lines
hw/i386/intel_iommu.c +17
-8+static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) +{int ret; +VTDContextEntry ce; +IOMMUNotifier *n; -if (ce) { -/* If the caller provided context entry, use it */ - ce_cache = *ce; - }else { -/* If the caller didn't provide ce, try to fetch */ - ret =vtd_dev_to_context_entry (s,pci_bus_num (vtd_as->bus), - vtd_as->devfn, &ce_cache); -if (ret) { + ret =vtd_dev_to_context_entry (vtd_as->iommu_state, +pci_bus_num (vtd_as->bus), + vtd_as->devfn, &ce); +if (ret) { +if (ret == -VTD_FR_CONTEXT_ENTRY_P ) { +/* +* It's a valid scenario to have a context entry that is +* not present. For example, when a device is removed +* from an existing domain then the context entry will be +* zeroed by the guest before it was put into another +* domain. When this happens, instead of synchronizing +* the shadow pages we should invalidate all existing +* mappings and notify the backends. hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1086
Quoted 10 lines
hw/i386/intel_iommu.c +5
-0+*/ + IOMMU_NOTIFIER_FOREACH(n, &vtd_as->iommu) { +vtd_address_space_unmap (vtd_as, n); + } + }else {/* * This should not really happen, but in case it happens, * we just skip the sync for this time. After all we even * don't have the root table pointer! */ It looks to me the comment is not accurate, no root pointer is not the only reason for the failure of
vtd_dev_to_context_entry().
Quoted 8 lines
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1071
Quoted 5 lines
error_report_once("%s: invalid context entry for bus 0x%x"hw/i386/intel_iommu.c +0
-4-" devfn 0x%x" , - __func__,pci_bus_num (vtd_as->bus), - vtd_as->devfn); -return 0 ;I'm not quite sure
error_report_once()is really needed here since all failures has been traced.
True; I'll then consider have all of them to be error_report_once() and drop the one here.
Quoted 11 lines
Quoted 11 lines
hw/i386/intel_iommu.c +3
-5+" devfn 0x%x" , __func__, + pci_bus_num(vtd_as->bus), vtd_as->devfn); } +return 0 ; } - info.domain_id =VTD_CONTEXT_ENTRY_DID (ce_cache.hi); - -return vtd_page_walk (&ce_cache, addr, addr + size, &info); -} -
Quoted 8 lines
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1095
Quoted 5 lines
hw/i386/intel_iommu.c +1
-3-static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) -{ -return vtd_sync_shadow_page_table_range (vtd_as,NULL ,0 ,UINT64_MAX ); +return vtd_sync_shadow_page_table_range (vtd_as, &ce,0 ,UINT64_MAX ); }As has been discussed, this will left addr
UINT64_MAX, it's better to have [start, end] instead of (start, range).
Hmm, this size is inclusive, so we should be fine. Though I'll take your advise to use start/end pair to be clearer.
Thanks!
Signature · 1 line
Peter Xu
Quoted 11 lines · Peter Xu
Quoted 8 lines
hw/i386/intel_iommu.c static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as, · line 1095
Quoted 5 lines
hw/i386/intel_iommu.c +1
-3-static int vtd_sync_shadow_page_table (VTDAddressSpace *vtd_as) -{ -return vtd_sync_shadow_page_table_range (vtd_as,NULL ,0 ,UINT64_MAX ); +return vtd_sync_shadow_page_table_range (vtd_as, &ce,0 ,UINT64_MAX ); }As has been discussed, this will left addr
UINT64_MAX, it's better to have [start, end] instead of (start, range).Hmm, this size is inclusive, so we should be fine. Though I'll take your advise to use start/end pair to be clearer.
Sorry it's not...
Actually vtd_page_walk() itself is not inclusive, so we will need to touch that up if we want to let the whole stack use the [start, end] inclusive way. However I would try not to bother with it since after all page sync operation is per-small-page granularity, so imho missing the last addr UINT64_MAX is ok (as long as we're covering the last page, which is UINT64_MAX - PAGE_SIZE + 1). I'm not sure whether it'll worth it then to change the whole stack into inclusive way... so I'll temporarily keep it as is.
Regards,
Signature · 1 line
Peter Xu