hw/mem/cxl_type3.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
When injecting a new poisoned region through qmp_cxl_inject_poison(),
the newly injected region should not overlap with existing poisoned
regions.
The current validation method does not consider the following
overlapping region:
┌───┬───────┬───┐
│a │ b(a) │a │
└───┴───────┴───┘
(a is a newly added region, b is an existing region, and b is a
subregion of a)
Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
---
hw/mem/cxl_type3.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 35ac59883a5b..8e32de327908 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -1331,9 +1331,7 @@ void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
ct3d = CXL_TYPE3(obj);
QLIST_FOREACH(p, &ct3d->poison_list, node) {
- if (((start >= p->start) && (start < p->start + p->length)) ||
- ((start + length > p->start) &&
- (start + length <= p->start + p->length))) {
+ if ((start < p->start + p->length) && (start + length > p->start)) {
error_setg(errp,
"Overlap with existing poisoned region not supported");
return;
--
2.37.3
On Thu, Jul 18, 2024 at 05:07:53AM -0400, Yao Xingtao wrote:
> <!DOCTYPE html><!-- BaNnErBlUrFlE-BoDy-start --><!-- Preheader Text : BEGIN --><meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;height:0px;max-height:0px;opacity:0;overflow:hidden;">
> When injecting a new poisoned region through qmp_cxl_inject_poison(), the newly injected region should not overlap with existing poisoned regions. The current validation method does not consider the following overlapping region: ┌───┬───────┬───┐
> </div>
> <!-- Preheader Text : END -->
>
> <!-- Email Banner : BEGIN -->
> <div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;height:0px;max-height:0px;opacity:0;overflow:hidden;">ZjQcmQRYFpfptBannerStart</div>
>
> <!--[if ((ie)|(mso))]>
> <table border="0" cellspacing="0" cellpadding="0" width="100%" style="padding: 16px 0px 16px 0px; direction: ltr" ><tr><td>
> <table border="0" cellspacing="0" cellpadding="0" style="padding: 0px 10px 5px 6px; width: 100%; border-radius:4px; border-top:4px solid #f32323;background-color:#F07B7B;"><tr><td valign="top">
> <table align="left" border="0" cellspacing="0" cellpadding="0" style="padding: 4px 8px 4px 8px">
> <tr><td style="color:#000000; font-family: 'Arial', sans-serif; font-weight:bold; font-size:14px; direction: ltr">
> This Message Is From an External Sender
> </td></tr>
> <tr><td style="color:#000000; font-weight:normal; font-family: 'Arial', sans-serif; font-size:12px; direction: ltr">
> Use caution opening files, clicking links or responding to requests.
> </td></tr>
>
> </table>
>
> </td></tr></table>
> </td></tr></table>
> <![endif]-->
>
> <![if !((ie)|(mso))]>
> <div dir="ltr" id="pfptBannergkdcmiu" style="all: revert !important; display:block !important; text-align: left !important; margin:16px 0px 16px 0px !important; padding:8px 16px 8px 16px !important; border-radius: 4px !important; min-width: 200px !important; background-color: #F07B7B !important; background-color: #F07B7B; border-top: 4px solid #f32323 !important; border-top: 4px solid #f32323;">
> <div id="pfptBannergkdcmiu" style="all: unset !important; float:left !important; display:block !important; margin: 0px 0px 1px 0px !important; max-width: 600px !important;">
> <div id="pfptBannergkdcmiu" style="all: unset !important; display:block !important; visibility: visible !important; background-color: #F07B7B !important; color:#000000 !important; color:#000000; font-family: 'Arial', sans-serif !important; font-family: 'Arial', sans-serif; font-weight:bold !important; font-weight:bold; font-size:14px !important; line-height:18px !important; line-height:18px">
> This Message Is From an External Sender
> </div>
> <div id="pfptBannergkdcmiu" style="all: unset !important; display:block !important; visibility: visible !important; background-color: #F07B7B !important; color:#000000 !important; color:#000000; font-weight:normal; font-family: 'Arial', sans-serif !important; font-family: 'Arial', sans-serif; font-size:12px !important; line-height:18px !important; line-height:18px; margin-top:2px !important;">
> Use caution opening files, clicking links or responding to requests.
> </div>
>
> </div>
>
> <div style="clear: both !important; display: block !important; visibility: hidden !important; line-height: 0 !important; font-size: 0.01px !important; height: 0px"> </div>
> </div>
> <![endif]>
>
> <div style="display:none !important;display:none;visibility:hidden;mso-hide:all;font-size:1px;color:#ffffff;line-height:1px;height:0px;max-height:0px;opacity:0;overflow:hidden;">ZjQcmQRYFpfptBannerEnd</div>
> <!-- Email Banner : END -->
>
> <!-- BaNnErBlUrFlE-BoDy-end -->
> <html>
> <head><!-- BaNnErBlUrFlE-HeAdEr-start -->
> <style>
> #pfptBannergkdcmiu { all: revert !important; display: block !important;
> visibility: visible !important; opacity: 1 !important;
> background-color: #F07B7B !important;
> max-width: none !important; max-height: none !important }
> .pfptPrimaryButtongkdcmiu:hover, .pfptPrimaryButtongkdcmiu:focus {
> background-color: #ef5b5b !important; }
> .pfptPrimaryButtongkdcmiu:active {
> background-color: #f32323 !important; }
> </style>
>
> <!-- BaNnErBlUrFlE-HeAdEr-end -->
> </head><body><pre style="font-family: sans-serif; font-size: 100%; white-space: pre-wrap; word-wrap: break-word">When injecting a new poisoned region through qmp_cxl_inject_poison(),
> the newly injected region should not overlap with existing poisoned
> regions.
>
> The current validation method does not consider the following
> overlapping region:
> ┌───┬───────┬───┐
> │a │ b(a) │a │
> └───┴───────┴───┘
> (a is a newly added region, b is an existing region, and b is a
> subregion of a)
>
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
> hw/mem/cxl_type3.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 35ac59883a5b..8e32de327908 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -1331,9 +1331,7 @@ void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> ct3d = CXL_TYPE3(obj);
>
> QLIST_FOREACH(p, &ct3d->poison_list, node) {
> - if (((start >= p->start) && (start < p->start + p->length)) ||
> - ((start + length > p->start) &&
> - (start + length <= p->start + p->length))) {
> + if ((start < p->start + p->length) && (start + length > p->start)) {
> error_setg(errp,
> "Overlap with existing poisoned region not supported");
> return;
As mentioned by Peter, we can use ranges_overlap() to improve the
code readability. Other than that, looks good t me.
btw, not sure only me or not, but the message does not display
correctly in mutt, seems not a plain text message, but looks fine in
outlook.
Fan
> --
> 2.37.3
>
> </pre></body></html>
> > > As mentioned by Peter, we can use ranges_overlap() to improve the > code readability. Other than that, looks good t me. > > btw, not sure only me or not, but the message does not display > correctly in mutt, seems not a plain text message, but looks fine in > outlook. I am not sure as well, but it shows properly on the mailing list. > > Fan
On Thu, 18 Jul 2024 05:07:53 -0400
Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote:
> When injecting a new poisoned region through qmp_cxl_inject_poison(),
> the newly injected region should not overlap with existing poisoned
> regions.
>
> The current validation method does not consider the following
> overlapping region:
> ┌───┬───────┬───┐
> │a │ b(a) │a │
> └───┴───────┴───┘
> (a is a newly added region, b is an existing region, and b is a
> subregion of a)
>
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
Looks correct to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huwei.com>
I've queued it on my local branch.
I need to put together an updated public one.
No huge rush to queue this up though I think as the effects
are minor.
Jonathan
> ---
> hw/mem/cxl_type3.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 35ac59883a5b..8e32de327908 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -1331,9 +1331,7 @@ void qmp_cxl_inject_poison(const char *path, uint64_t start, uint64_t length,
> ct3d = CXL_TYPE3(obj);
>
> QLIST_FOREACH(p, &ct3d->poison_list, node) {
> - if (((start >= p->start) && (start < p->start + p->length)) ||
> - ((start + length > p->start) &&
> - (start + length <= p->start + p->length))) {
> + if ((start < p->start + p->length) && (start + length > p->start)) {
> error_setg(errp,
> "Overlap with existing poisoned region not supported");
> return;
On Thu, 18 Jul 2024 at 17:37, Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > > On Thu, 18 Jul 2024 05:07:53 -0400 > Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote: > > > When injecting a new poisoned region through qmp_cxl_inject_poison(), > > the newly injected region should not overlap with existing poisoned > > regions. > > > > The current validation method does not consider the following > > overlapping region: > > ┌───┬───────┬───┐ > > │a │ b(a) │a │ > > └───┴───────┴───┘ > > (a is a newly added region, b is an existing region, and b is a > > subregion of a) > > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > Looks correct to me. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huwei.com> > I've queued it on my local branch. > I need to put together an updated public one. > > No huge rush to queue this up though I think as the effects > are minor. I think you can probably write this as ranges_overlap(start, len, p->start, p->length) using the utility function in include/qemu/ranges.h, which is a bit more readable than open-coding the overlap test. (There's another couple of open-coded overlap tests in cxl-mailbox-utils.c.) thanks -- PMM
> -----Original Message----- > From: Peter Maydell <peter.maydell@linaro.org> > Sent: Friday, July 19, 2024 1:12 AM > To: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; fan.ni@samsung.com; > qemu-devel@nongnu.org > Subject: Re: [PATCH] mem/cxl_type3: Fix overlapping region validation error > > On Thu, 18 Jul 2024 at 17:37, Jonathan Cameron via > <qemu-devel@nongnu.org> wrote: > > > > On Thu, 18 Jul 2024 05:07:53 -0400 > > Yao Xingtao <yaoxt.fnst@fujitsu.com> wrote: > > > > > When injecting a new poisoned region through qmp_cxl_inject_poison(), > > > the newly injected region should not overlap with existing poisoned > > > regions. > > > > > > The current validation method does not consider the following > > > overlapping region: > > > ┌───┬───────┬───┐ > > > │a │ b(a) │a │ > > > └───┴───────┴───┘ > > > (a is a newly added region, b is an existing region, and b is a > > > subregion of a) > > > > > > Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> > > Looks correct to me. > > > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huwei.com> > > I've queued it on my local branch. > > I need to put together an updated public one. > > > > No huge rush to queue this up though I think as the effects > > are minor. > > I think you can probably write this as > ranges_overlap(start, len, p->start, p->length) > using the utility function in include/qemu/ranges.h, which is > a bit more readable than open-coding the overlap test. Great! I will fix it in the next revision. > > (There's another couple of open-coded overlap tests in > cxl-mailbox-utils.c.) I will collect these issues and fix them in separate patches. > > thanks > -- PMM
© 2016 - 2024 Red Hat, Inc.