Re: [PATCH] exec: Add RISC-V WorldGuard WID field to MemTxAttrs

["Alex Bennée" <alex.bennee@linaro.org>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 17 Jul 2026 at 11:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>> > On 18/3/26 05:40, Jim Shu wrote:
>> >> On Tue, Feb 10, 2026 at 8:25 AM Richard Henderson
>> >> <richard.henderson@linaro.org> wrote:
>> >> ...
>> >>> Hmm.  This really overlaps the secure and space fields from arm, and possibly some of the
>> >>> others as well (e.g. user, requester_id, pid).
>> >>>
>> >>> I don't really have a good suggestion for that right now, but it would be nice to not keep
>> >>> expanding the count of these sorts of fields that somehow specify the originator, but
>> >>> clearly cannot overlap.
>> >>>
>> >>> I'm reasonably sure we've had this discussion before, but nothing has come of it.
>> >>>
>> >>> Time to paint the bikeshed again?
>> >
>> > Last discussion IIRC:
>> > https://lore.kernel.org/qemu-devel/CAFEAcA8vKNkfKgp_Yymo9NA1=E2XJYXAMTgO3z6q6DHgqkAwRw@mail.gmail.com/
>> >
>> > (see also a suggestion in
>> > https://lore.kernel.org/qemu-devel/Z4+P3eHXqcU4Dqdx@intel.com/)
>>
>> Also somewhat related:
>>
>>   Message-Id: <20221111182535.64844-1-alex.bennee@linaro.org>
>>   Date: Fri, 11 Nov 2022 18:25:15 +0000
>>   Subject: [PATCH for 8.0 v5 00/20] use MemTxAttrs to avoid current_cpu in hw/
>>
>> Another case that was mentioned in a Core Collective meeting was
>> handling MMIO devices with IOMMUs (currently requester_id is tied to
>> PCI). I guess the WorldGuard WID field is a similar thing.
>
> The worldguard ID is more like the Arm security space field, as I
> understand it -- it encodes what the request should or should
> not be able to access, and multiple different transaction masters
> might be able to send with the same worldguard ID. A requester_id
> on the other hand is intended to identify a unique sender.
> (In AXI these things turn up in different signals.)
>
> I think the trick with requester_id is that we need to identify
> what we need to encode here and make sure we don't confuse things.
> (e.g. a CPU needs to not be able to emit something that looks like
> a PCI request by accident, because some devices need to be able
> to tell "this really did come from a PCI device" from "this is a
> CPU doing a normal load/store insn"). So we probably want some
> kind of "this is what the requester_id is" enum rather than just
> a convention.

Yeah I had a type:

  /**
   * typedef MemTxRequesterType - source of memory transaction
   *
   * Every memory transaction comes from a specific place which defines
   * how requester_id should be handled if at all.
   *
   * UNSPECIFIED: the default for otherwise undefined MemTxAttrs
   * CPU: requester_id is the global cpu_index
   *      This needs further processing if you need to work out which
   *      socket or complex it comes from
   * PCI: indicates the requester_id is a PCI id
   * MACHINE: indicates a machine specific encoding
   *          This will require further processing to decode into its
   *          constituent parts.
   */
  typedef enum MemTxRequesterType {
      MTRT_UNSPECIFIED = 0,
      MTRT_CPU,
      MTRT_PCI,
      MTRT_MACHINE
  } MemTxRequesterType;

  /**
   * typedef MemTxAttrs - attributes of a memory transaction
   *
   * Every memory transaction has associated with it a set of
   * attributes. Some of these are generic (such as the ID of
   * the bus master); some are specific to a particular kind of
   * bus (such as the ARM Secure/NonSecure bit). We define them
   * all as non-overlapping bitfields in a single struct to avoid
   * confusion if different parts of QEMU used the same bit for
   * different semantics.
   */
  typedef struct MemTxAttrs {
      /* Requester type (e.g. CPU or PCI MSI) */
      MemTxRequesterType requester_type:2;
      /* Requester ID */
      unsigned int requester_id:16;
      /*
       * ARM/AMBA: TrustZone Secure access
       * x86: System Management Mode access
       */
      unsigned int secure:1;
      /*
       * ARM: ArmSecuritySpace.  This partially overlaps secure, but it is
       * easier to have both fields to assist code that does not understand
       * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
       */
      unsigned int space:2;
      /* Memory access is usermode (unprivileged) */
      unsigned int user:1;
      /*
       * Bus interconnect and peripherals can access anything (memories,
       * devices) by default. By setting the 'memory' bit, bus transaction
       * are restricted to "normal" memories (per the AMBA documentation)
       * versus devices. Access to devices will be logged and rejected
       * (see MEMTX_ACCESS_ERROR).
       */
      unsigned int memory:1;
      /* Debug access that can even write to ROM. */
      unsigned int debug:1;
      /*
       * PID (PCI PASID) support: Limited to 8 bits process identifier.
       */
      unsigned int pid:8;

      /* PCI - IOMMU operations, see PCIAddressType */
      unsigned int address_type:1;

      uint8_t _reserved1;
      uint16_t _reserved2;
  } MemTxAttrs;

But I suspect MTRT_MACHINE might be a bit too much of a blunt
instrument. I never used in my series but the idea is it would call back
to the machine to work out how it interpreted requester_id.

>
> thanks
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro