[PATCH v2 -qemu 0/2] hw/cxl: Media Operation Discovery fixlets

[PATCH v2 -qemu 0/2] hw/cxl: Media Operation Discovery fixlets

[Davidlohr Bueso]

Hello,

Changes from v1: cleaned up comments in patch 2.

While working on the driver side of this, I tripped on two qemu issues
around discovery semantics. I think this is the first time these code
paths are exercised.

Applies against 'cxl-2026-01-09-draft'.

Thanks!

Davidlohr Bueso (2):
  hw/cxl: Respect Media Operation max ops discovery semantics
  hw/cxl: Exclude Discovery from Media Operation Discovery output

 hw/cxl/cxl-mailbox-utils.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

--
2.39.5

[PATCH 1/2] hw/cxl: Respect Media Operation max ops discovery semantics

[Davidlohr Bueso]

The Discovery handler rejects requests where start_index + num_ops
exceeds the total number of supported operations. Per CXL 4.0
Table 8-332, num_ops is the "Maximum number of Media Operation to
return" - a maximum, not an exact count. The device should return
up to that many entries, not reject the request.

Cap num_ops to the available entries from start_index instead of
erroring the command.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 hw/cxl/cxl-mailbox-utils.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index a3143f3faa23..71a012121c87 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -2593,6 +2593,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
     } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in;
     struct media_op_discovery_out_pl *media_out_pl =
         (struct media_op_discovery_out_pl *)payload_out;
+    int total = ARRAY_SIZE(media_op_matrix);
     int num_ops, start_index, i;
     int count = 0;
 
@@ -2609,24 +2610,20 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
      * sub class command.
      */
     if (media_op_in_disc_pl->dpa_range_count ||
-        start_index + num_ops > ARRAY_SIZE(media_op_matrix)) {
+        start_index >= total) {
         return CXL_MBOX_INVALID_INPUT;
     }
 
     media_out_pl->dpa_range_granularity = CXL_CACHE_LINE_SIZE;
-    media_out_pl->total_supported_operations =
-                                     ARRAY_SIZE(media_op_matrix);
-    if (num_ops > 0) {
-        for (i = start_index; i < start_index + num_ops; i++) {
-            media_out_pl->entry[count].media_op_class =
-                    media_op_matrix[i].media_op_class;
-            media_out_pl->entry[count].media_op_subclass =
-                        media_op_matrix[i].media_op_subclass;
-            count++;
-            if (count == num_ops) {
-                break;
-            }
-        }
+    media_out_pl->total_supported_operations = total;
+
+    num_ops = MIN(num_ops, total - start_index);
+    for (i = 0; i < num_ops; i++) {
+        media_out_pl->entry[count].media_op_class =
+                media_op_matrix[start_index + i].media_op_class;
+        media_out_pl->entry[count].media_op_subclass =
+                    media_op_matrix[start_index + i].media_op_subclass;
+        count++;
     }
 
     media_out_pl->num_of_supported_operations = count;
-- 
2.39.5

[PATCH 2/2] hw/cxl: Exclude Discovery from Media Operation Discovery output

[Davidlohr Bueso]

Per CXL 4.0 Table 8-331, the Discovery operation "returns a list of
all Media Operations that the device supports, with the exception of
the Discovery operation (Class=0, Subclass=0)."

Filter out Discovery entries when building the output list and adjust
total_supported_operations accordingly.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 hw/cxl/cxl-mailbox-utils.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 71a012121c87..6932db963788 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -2593,7 +2593,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
     } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in;
     struct media_op_discovery_out_pl *media_out_pl =
         (struct media_op_discovery_out_pl *)payload_out;
-    int total = ARRAY_SIZE(media_op_matrix);
+    int total = ARRAY_SIZE(media_op_matrix) - 1; /* exclude Discovery */
     int num_ops, start_index, i;
     int count = 0;
 
@@ -2619,10 +2619,12 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
 
     num_ops = MIN(num_ops, total - start_index);
     for (i = 0; i < num_ops; i++) {
+        int idx = start_index + i + 1; /* skip Discovery (first entry) */
+
         media_out_pl->entry[count].media_op_class =
-                media_op_matrix[start_index + i].media_op_class;
+                media_op_matrix[idx].media_op_class;
         media_out_pl->entry[count].media_op_subclass =
-                    media_op_matrix[start_index + i].media_op_subclass;
+                    media_op_matrix[idx].media_op_subclass;
         count++;
     }
 
-- 
2.39.5

Re: [PATCH 1/2] hw/cxl: Respect Media Operation max ops discovery semantics

[Jonathan Cameron via qemu development]

On Wed, 18 Mar 2026 11:55:07 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> The Discovery handler rejects requests where start_index + num_ops
> exceeds the total number of supported operations. Per CXL 4.0
> Table 8-332, num_ops is the "Maximum number of Media Operation to
> return" - a maximum, not an exact count. The device should return
> up to that many entries, not reject the request.
> 
> Cap num_ops to the available entries from start_index instead of
> erroring the command.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
One trivial thing inline.

> ---
>  hw/cxl/cxl-mailbox-utils.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index a3143f3faa23..71a012121c87 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -2593,6 +2593,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
>      } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in;
>      struct media_op_discovery_out_pl *media_out_pl =
>          (struct media_op_discovery_out_pl *)payload_out;
> +    int total = ARRAY_SIZE(media_op_matrix);
>      int num_ops, start_index, i;
>      int count = 0;
>  
> @@ -2609,24 +2610,20 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
>       * sub class command.
>       */
>      if (media_op_in_disc_pl->dpa_range_count ||
> -        start_index + num_ops > ARRAY_SIZE(media_op_matrix)) {
> +        start_index >= total) {
>          return CXL_MBOX_INVALID_INPUT;
>      }
>  
>      media_out_pl->dpa_range_granularity = CXL_CACHE_LINE_SIZE;
> -    media_out_pl->total_supported_operations =
> -                                     ARRAY_SIZE(media_op_matrix);
> -    if (num_ops > 0) {
> -        for (i = start_index; i < start_index + num_ops; i++) {
> -            media_out_pl->entry[count].media_op_class =
> -                    media_op_matrix[i].media_op_class;
> -            media_out_pl->entry[count].media_op_subclass =
> -                        media_op_matrix[i].media_op_subclass;
> -            count++;
> -            if (count == num_ops) {
> -                break;
> -            }
> -        }
> +    media_out_pl->total_supported_operations = total;
> +
> +    num_ops = MIN(num_ops, total - start_index);
> +    for (i = 0; i < num_ops; i++) {
> +        media_out_pl->entry[count].media_op_class =
> +                media_op_matrix[start_index + i].media_op_class;
> +        media_out_pl->entry[count].media_op_subclass =
> +                    media_op_matrix[start_index + i].media_op_subclass;
Patch in general looks good, but the indenting here is a bit too creative
/ inconsistent.
        media_out_pl->entry[count].media_op_class =
                media_op_matrix[start_index + i].media_op_class;
        media_out_pl->entry[count].media_op_subclass =
                media_op_matrix[start_index + i].media_op_subclass;

or something along those lines.

> +        count++;
>      }
>  
>      media_out_pl->num_of_supported_operations = count;

Re: [PATCH 2/2] hw/cxl: Exclude Discovery from Media Operation Discovery output

[Jonathan Cameron via qemu development]

On Wed, 18 Mar 2026 11:55:08 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> Per CXL 4.0 Table 8-331, the Discovery operation "returns a list of
> all Media Operations that the device supports, with the exception of
> the Discovery operation (Class=0, Subclass=0)."
> 
> Filter out Discovery entries when building the output list and adjust
> total_supported_operations accordingly.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Both could do with a fixes tag.

> ---
>  hw/cxl/cxl-mailbox-utils.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 71a012121c87..6932db963788 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -2593,7 +2593,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
>      } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in;
>      struct media_op_discovery_out_pl *media_out_pl =
>          (struct media_op_discovery_out_pl *)payload_out;
> -    int total = ARRAY_SIZE(media_op_matrix);
> +    int total = ARRAY_SIZE(media_op_matrix) - 1; /* exclude Discovery */
>      int num_ops, start_index, i;
>      int count = 0;
>  
> @@ -2619,10 +2619,12 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in,
>  
>      num_ops = MIN(num_ops, total - start_index);
>      for (i = 0; i < num_ops; i++) {
> +        int idx = start_index + i + 1; /* skip Discovery (first entry) */
> +
>          media_out_pl->entry[count].media_op_class =
> -                media_op_matrix[start_index + i].media_op_class;
> +                media_op_matrix[idx].media_op_class;
>          media_out_pl->entry[count].media_op_subclass =
> -                    media_op_matrix[start_index + i].media_op_subclass;
> +                    media_op_matrix[idx].media_op_subclass;
Indent comment follows through.

Otherwise LGTM.

I'll queue it up locally but if you want to send a new version and with mst@redhat.com
in the to, given it's fixes and where we are in the cycle they may get picked up sooner.

If you do that, 
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com> 
for both patches

Whilst I don't think anything else I have queued touches this code, make sure
you base on upstream qemu.

Thanks,

Jonathan



>          count++;
>      }
>