Re: [PATCH v1 04/17] xen/riscv: introduce device-agnostic MMIO emulation dispatch

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/10/26 4:49 PM, Baptiste Le Duc wrote:
> > diff --git a/xen/arch/riscv/include/asm/mmio.h 
> > b/xen/arch/riscv/include/asm/mmio.h
> > new file mode 100644
> > index 0000000000..18df1133e6
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/mmio.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> According to coding style, it should be GPL-2.0-only.
Could you please point me to the line in the coding style document where 
this is mentioned?
If you are referring to:
  New files should start with a single-line SPDX comment to express the
  license, e.g.:

  /* SPDX-License-Identifier: GPL-2.0-only */

  See LICENSES/ for a list of licenses and SPDX tags currently used.

Then my understanding is that /* SPDX-License-Identifier: GPL-2.0-only 
*/ is used only as an example, and I can choose any license from 
LICENSES/. There, it is mentioned:
  Valid-License-Identifier: LGPL-2.0-only
  Valid-License-Identifier: LGPL-2.0-or-later

I am pretty sure that I am free to choose any license that does not 
conflict with the other licenses used in the project.
> > +#ifndef RISCV_MMIO_H
> > +#define RISCV_MMIO_H
> > +
> > +#include <xen/lib.h>
> > +#include <xen/rwlock.h>
> > +
> > +#define MAX_IO_HANDLER  16
> > +
> > +typedef struct {
> > +    paddr_t gpa;
> > +    unsigned int len;  /* access width in bytes (1, 2, 4, 8) */
> > +    bool is_write;
> > +    register_t data;   /* store: value to write; load: value read (set by 
> > handler) */
> Nit: line too long (85)
I will apply that. Actually I've already fixed that by putting the 
comment above:
  /* store: value to write; load: value read (set by handler) */
  register_t data;


> > diff --git a/xen/arch/riscv/mmio.c b/xen/arch/riscv/mmio.c
> > new file mode 100644
> > index 0000000000..7d56bc8b27
> > --- /dev/null
> > +++ b/xen/arch/riscv/mmio.c
> > @@ -0,0 +1,145 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> Should be GPL-2.0-only.
Regarding license I've wrote a comment above so lets continue discussion 
there.
> > +/*
> > + * Copyright (C) Vates
> > + */
> Why have you included a copyright notice here, but not in the other
> files? 
So I just decided to do that for new files as I am not using corporate 
e-mail.
I don’t know if you can keep it,

Good point, I have to ask then someone from our legal department...

 but I just wanted to point out
> that there are other files where this type of copyright notice includes
> the year.
Before, I used to include the year, but someone pointed out (or perhaps 
I misunderstood) that there isn’t much point in including it and that it 
is enough to have just (c) <company name>.
Thanks.

~ Oleksii