From: Jonathan Cameron via <qemu-devel@nongnu.org>
To: <qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>,
Michael Tsirkin <mst@redhat.com>,
Michael Tokarev <mjt@tls.msk.ru>
Cc: linuxarm@huawei.com, "Fan Ni" <fan.ni@samsung.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: [PATCH v5 2/5] hw/cxl: Use switch statements for read and write of cachemem registers
Date: Mon, 23 Oct 2023 15:02:07 +0100 [thread overview]
Message-ID: <20231023140210.3089-3-Jonathan.Cameron@huawei.com> (raw)
In-Reply-To: <20231023140210.3089-1-Jonathan.Cameron@huawei.com>
Establishing that only register accesses of size 4 and 8 can occur
using these functions requires looking at their callers. Make it
easier to see that by using switch statements.
Assertions are used to enforce that the register storage is of the
matching size, allowing fixed values to be used for divisors of
the array indices.
Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
---
v5: _Static_assert() is a declaration in C, so may not follow
a label. Hence wrap it in {} to ensure it doesn't.
Issue seen with clang builds (and in some cases at least older
GCC).
We can't wrap it in {} in the QEMU_BUILD_BUG_ON() define as that
is used in a few places outside of functions.
hw/cxl/cxl-component-utils.c | 66 +++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 23 deletions(-)
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index f3bbf0fd13..9d4f4bc8d4 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -67,16 +67,24 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr offset,
CXLComponentState *cxl_cstate = opaque;
ComponentRegisters *cregs = &cxl_cstate->crb;
- if (size == 8) {
+ switch (size) {
+ case 4:
+ if (cregs->special_ops && cregs->special_ops->read) {
+ return cregs->special_ops->read(cxl_cstate, offset, 4);
+ } else {
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
+ return cregs->cache_mem_registers[offset / 4];
+ }
+ case 8:
qemu_log_mask(LOG_UNIMP,
"CXL 8 byte cache mem registers not implemented\n");
return 0;
- }
-
- if (cregs->special_ops && cregs->special_ops->read) {
- return cregs->special_ops->read(cxl_cstate, offset, size);
- } else {
- return cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)];
+ default:
+ /*
+ * In line with specifiction limitaions on access sizes, this
+ * routine is not called with other sizes.
+ */
+ g_assert_not_reached();
}
}
@@ -117,25 +125,37 @@ static void cxl_cache_mem_write_reg(void *opaque, hwaddr offset, uint64_t value,
ComponentRegisters *cregs = &cxl_cstate->crb;
uint32_t mask;
- if (size == 8) {
- qemu_log_mask(LOG_UNIMP,
- "CXL 8 byte cache mem registers not implemented\n");
+ switch (size) {
+ case 4: {
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_regs_write_mask) != 4);
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
+ mask = cregs->cache_mem_regs_write_mask[offset / 4];
+ value &= mask;
+ /* RO bits should remain constant. Done by reading existing value */
+ value |= ~mask & cregs->cache_mem_registers[offset / 4];
+ if (cregs->special_ops && cregs->special_ops->write) {
+ cregs->special_ops->write(cxl_cstate, offset, value, size);
+ return;
+ }
+
+ if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
+ offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
+ dumb_hdm_handler(cxl_cstate, offset, value);
+ } else {
+ cregs->cache_mem_registers[offset / 4] = value;
+ }
return;
}
- mask = cregs->cache_mem_regs_write_mask[offset / sizeof(*cregs->cache_mem_regs_write_mask)];
- value &= mask;
- /* RO bits should remain constant. Done by reading existing value */
- value |= ~mask & cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)];
- if (cregs->special_ops && cregs->special_ops->write) {
- cregs->special_ops->write(cxl_cstate, offset, value, size);
+ case 8:
+ qemu_log_mask(LOG_UNIMP,
+ "CXL 8 byte cache mem registers not implemented\n");
return;
- }
-
- if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
- offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
- dumb_hdm_handler(cxl_cstate, offset, value);
- } else {
- cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)] = value;
+ default:
+ /*
+ * In line with specifiction limitaions on access sizes, this
+ * routine is not called with other sizes.
+ */
+ g_assert_not_reached();
}
}
--
2.39.2
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: <qemu-devel@nongnu.org>, <linux-cxl@vger.kernel.org>,
Michael Tsirkin <mst@redhat.com>,
Michael Tokarev <mjt@tls.msk.ru>
Cc: linuxarm@huawei.com, "Fan Ni" <fan.ni@samsung.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: [PATCH v5 2/5] hw/cxl: Use switch statements for read and write of cachemem registers
Date: Mon, 23 Oct 2023 15:02:07 +0100 [thread overview]
Message-ID: <20231023140210.3089-3-Jonathan.Cameron@huawei.com> (raw)
Message-ID: <20231023140207.jGHgQlF62G5eW9ZDR9fV1CUXGJrc9LCMTpLye3Zq_4E@z> (raw)
In-Reply-To: <20231023140210.3089-1-Jonathan.Cameron@huawei.com>
Establishing that only register accesses of size 4 and 8 can occur
using these functions requires looking at their callers. Make it
easier to see that by using switch statements.
Assertions are used to enforce that the register storage is of the
matching size, allowing fixed values to be used for divisors of
the array indices.
Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
---
v5: _Static_assert() is a declaration in C, so may not follow
a label. Hence wrap it in {} to ensure it doesn't.
Issue seen with clang builds (and in some cases at least older
GCC).
We can't wrap it in {} in the QEMU_BUILD_BUG_ON() define as that
is used in a few places outside of functions.
hw/cxl/cxl-component-utils.c | 66 +++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 23 deletions(-)
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index f3bbf0fd13..9d4f4bc8d4 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -67,16 +67,24 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr offset,
CXLComponentState *cxl_cstate = opaque;
ComponentRegisters *cregs = &cxl_cstate->crb;
- if (size == 8) {
+ switch (size) {
+ case 4:
+ if (cregs->special_ops && cregs->special_ops->read) {
+ return cregs->special_ops->read(cxl_cstate, offset, 4);
+ } else {
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
+ return cregs->cache_mem_registers[offset / 4];
+ }
+ case 8:
qemu_log_mask(LOG_UNIMP,
"CXL 8 byte cache mem registers not implemented\n");
return 0;
- }
-
- if (cregs->special_ops && cregs->special_ops->read) {
- return cregs->special_ops->read(cxl_cstate, offset, size);
- } else {
- return cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)];
+ default:
+ /*
+ * In line with specifiction limitaions on access sizes, this
+ * routine is not called with other sizes.
+ */
+ g_assert_not_reached();
}
}
@@ -117,25 +125,37 @@ static void cxl_cache_mem_write_reg(void *opaque, hwaddr offset, uint64_t value,
ComponentRegisters *cregs = &cxl_cstate->crb;
uint32_t mask;
- if (size == 8) {
- qemu_log_mask(LOG_UNIMP,
- "CXL 8 byte cache mem registers not implemented\n");
+ switch (size) {
+ case 4: {
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_regs_write_mask) != 4);
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
+ mask = cregs->cache_mem_regs_write_mask[offset / 4];
+ value &= mask;
+ /* RO bits should remain constant. Done by reading existing value */
+ value |= ~mask & cregs->cache_mem_registers[offset / 4];
+ if (cregs->special_ops && cregs->special_ops->write) {
+ cregs->special_ops->write(cxl_cstate, offset, value, size);
+ return;
+ }
+
+ if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
+ offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
+ dumb_hdm_handler(cxl_cstate, offset, value);
+ } else {
+ cregs->cache_mem_registers[offset / 4] = value;
+ }
return;
}
- mask = cregs->cache_mem_regs_write_mask[offset / sizeof(*cregs->cache_mem_regs_write_mask)];
- value &= mask;
- /* RO bits should remain constant. Done by reading existing value */
- value |= ~mask & cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)];
- if (cregs->special_ops && cregs->special_ops->write) {
- cregs->special_ops->write(cxl_cstate, offset, value, size);
+ case 8:
+ qemu_log_mask(LOG_UNIMP,
+ "CXL 8 byte cache mem registers not implemented\n");
return;
- }
-
- if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
- offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
- dumb_hdm_handler(cxl_cstate, offset, value);
- } else {
- cregs->cache_mem_registers[offset / sizeof(*cregs->cache_mem_registers)] = value;
+ default:
+ /*
+ * In line with specifiction limitaions on access sizes, this
+ * routine is not called with other sizes.
+ */
+ g_assert_not_reached();
}
}
--
2.39.2
next prev parent reply other threads:[~2023-10-23 14:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-23 14:02 [PATCH v5 0/5] hw/cxl: Line length reduction and related Jonathan Cameron via
2023-10-23 14:02 ` Jonathan Cameron
2023-10-23 14:02 ` [PATCH v5 1/5] hw/cxl: Use a switch to explicitly check size in caps_reg_read() Jonathan Cameron via
2023-10-23 14:02 ` Jonathan Cameron
2023-10-23 14:02 ` Jonathan Cameron via [this message]
2023-10-23 14:02 ` [PATCH v5 2/5] hw/cxl: Use switch statements for read and write of cachemem registers Jonathan Cameron
2023-10-23 14:02 ` [PATCH v5 3/5] hw/cxl: CXLDVSECPortExtensions renamed to CXLDVSECPortExt Jonathan Cameron via
2023-10-23 14:02 ` Jonathan Cameron
2023-10-23 14:02 ` [PATCH v5 4/5] hw/cxl: Line length reductions Jonathan Cameron via
2023-10-23 14:02 ` Jonathan Cameron
2023-10-23 14:02 ` [PATCH v5 5/5] hw/cxl: Fix a QEMU_BUILD_BUG_ON() in switch statement scope issue Jonathan Cameron via
2023-10-23 14:02 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231023140210.3089-3-Jonathan.Cameron@huawei.com \
--to=qemu-devel@nongnu.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=fan.ni@samsung.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linuxarm@huawei.com \
--cc=mjt@tls.msk.ru \
--cc=mst@redhat.com \
--cc=philmd@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).