On 9/27/22 07:14, Alex Bennée wrote:
+/*
+ * Every memory transaction comes from a specific place which defines
+ * how requester_id should be handled. For CPU's the requester_id is
+ * the global cpu_index which needs further processing if you need to
+ * work out which socket or complex it comes from. UNSPECIFIED is the
+ * default for otherwise undefined MemTxAttrs. PCI indicates the
+ * requester_id is a PCI id. MACHINE indicates a machine specific
+ * encoding which needs further processing to decode into its
+ * constituent parts.
+ */
+typedef enum MemTxRequesterType {
+ MTRT_CPU = 0,
+ MTRT_UNSPECIFIED,
+ MTRT_PCI,
+ MTRT_MACHINE
+} MemTxRequesterType;
I wonder if UNSPECIFIED should be zero, or even ILLEGAL, attempting to catch MemTxAttrs
foo = { }, which is now ill-formed as it doesn't initialize .requester_id to match CPU.
Perhaps a preceeding patch, should introduce
#define MEMTXATTRS_CPU(cpu) ((MemTxAttrs){ })
and modify all uses of "= { }", at least under target/.
--- a/hw/misc/tz-msc.c
+++ b/hw/misc/tz-msc.c
@@ -137,11 +137,11 @@ static MemTxResult tz_msc_read(void *opaque, hwaddr addr,
uint64_t *pdata,
return MEMTX_OK;
case MSCAllowSecure:
attrs.secure = 1;
- attrs.unspecified = 0;
+ attrs.requester_type = MTRT_CPU;
break;
case MSCAllowNonSecure:
attrs.secure = 0;
- attrs.unspecified = 0;
+ attrs.requester_type = MTRT_CPU;
break;
This is surely incomplete. You can't just set "cpu" without saying where it's
from.
Since this device is only used by the ARMSSE machine, I would hope that attrs.unspecified
should never be set before the patch, and thus MTRT_CPU should be set afterward.
r~
