[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 11/20] xen/riscv: introduce per-vCPU IMSIC state





On 8/4/26 5:48 PM, Oleksii Kurochko wrote:
Each vCPU interacting with the IMSIC requires state to track the
associated guest interrupt file and its backing context.

Introduce a per-vCPU structure to hold IMSIC-related state, including
the guest interrupt file identifier and the CPU providing the backing
VS-file. Access to the guest file identifier is protected by a lock.

Initialize this structure during vCPU setup and store it in arch_vcpu.
The initial state marks the VS-file as software-backed until it becomes
associated with a physical CPU.

Add helper to retrieve the guest interrupt file identifier:
- vcpu_guest_file_id() is going to be used during update of APLIC's
   target register with the pair of information <guest_file_id, cpu_id>
   (to have MSI delivery mode work properly) when guest is trying to
   access vAPLIC's target register.
It will be used in the follow up patches.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Changes in v6-7:
  - Nothing changed. Only rebase.
---
Changes in v5:
  - Move v->arch.vimsic_state = imsic_state; after full initialization of
    the struct, so the pointer only becomes globally visible once all
    fields are set up.
  - Add Acked-by: Jan Beulich <jbeulich@xxxxxxxx>.
---
Changes in v4:
-  s/w vs h/w IMSIC VS-file commentary for struct vimsic_state:
    - fix the vsfile_pcpu h/w condition:
      "vsfile_pcpu >= 0" -> "vsfile_pcpu < NR_CPUS"
      (the old wording conflicted with the s/w "== NR_CPUS" case).
    - reorder both comment blocks to the "s/w ... / h/w ..." form for 
readability.
  - drop IMPOSSIBLE_GUEST_FILE_ID: the s/w IMSIC VS-file is always available
    and corresponds to guest_file_id == 0, which xvzalloc() already provides,
    so the explicit initializer in vcpu_imsic_init() and the macro itself
    are unneeded.
---
Changes in v3:
  - Drop const from imsic_set_guest_file_id() and vcpu_imsic_deinit() as
    it only works due to vimsic_state being a pointer member.
  - Use XVFREE() in vcpu_imsic_deinit() to make it idempotent.
  - Fix SW-file typo in struct vimsic_state comments; should be VS-file.
  - Drop imsic_set_guest_file_id() here, it will be added later when it
    will be nessary to initialise guest file id as the correspondendt code
    in this patch series was reworked and there is no need to use this
    function in arch_vcpu_create().
  - Introduce IMPOSSIBLE_GUEST_FILE_ID and init with it ->guest_file_id.
---
Changes in v2:
  - Rename imsic_state to vimsic_state.
  - Use 'unsigned int' for vsfile_pcpu.
  - Drop initialzation of ->guest_file_id as it will be by default zero.
  - Add the comment about ->guest_file_id field.
  - Drop __init for vcpu_imsic_init() as it could be used during post-boot
    vCPU creation.
  - Update the commit message.
  - Drop locks around ->guest_file_id() in  vcpu_guest_file_id() and 
imsic_set_guest_file_id().
---
---
  xen/arch/riscv/imsic.c              | 35 +++++++++++++++++++++++++++++
  xen/arch/riscv/include/asm/domain.h |  2 ++
  xen/arch/riscv/include/asm/imsic.h  | 22 ++++++++++++++++++
  3 files changed, 59 insertions(+)

diff --git a/xen/arch/riscv/imsic.c b/xen/arch/riscv/imsic.c
index f7b70a8da09e..5a5758e45dc2 100644
--- a/xen/arch/riscv/imsic.c
+++ b/xen/arch/riscv/imsic.c
@@ -16,6 +16,7 @@
  #include <xen/errno.h>
  #include <xen/init.h>
  #include <xen/macros.h>
+#include <xen/sched.h>
  #include <xen/smp.h>
  #include <xen/spinlock.h>
  #include <xen/xvmalloc.h>
@@ -56,6 +57,11 @@ do {                            \
      csr_clear(CSR_SIREG, v);    \
  } while (0)
+unsigned int vcpu_guest_file_id(const struct vcpu *v)
+{
+    return ACCESS_ONCE(v->arch.vimsic_state->guest_file_id);
+}
+
  void __init imsic_ids_local_delivery(bool enable)
  {
      if ( enable )
@@ -312,6 +318,35 @@ static int imsic_parse_node(const struct dt_device_node 
*node,
      return 0;
  }
+int vcpu_imsic_init(struct vcpu *v)
+{
+    struct vimsic_state *imsic_state;
+
+    /* Allocate IMSIC context */
+    imsic_state = xvzalloc(struct vimsic_state);
+    if ( !imsic_state )
+        return -ENOMEM;
+
+    /* Setup IMSIC context  */
+    rwlock_init(&imsic_state->vsfile_lock);
+
+    /*
+     * xvzalloc() already cleared the context, so guest_file_id == 0, i.e. the
+     * always-available s/w IMSIC VS-file. Only vsfile_pcpu needs an explicit
+     * initializer as its s/w VS-file value is NR_CPUS rather than 0.
+     */
+    imsic_state->vsfile_pcpu = NR_CPUS;
+
Considering our conversation in another patch series vsfile_cpu would be better name. Don't you mind if I will change vsfile_pcpu -> vsfile_cpu and everywhere it is needed in this patch with saving of your Acked-by?

~ Oleksii



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.