2023-11-22 12:49:36

by Christian Brauner

[permalink] [raw]
Subject: [PATCH v2 1/4] i915: make inject_virtual_interrupt() void

The single caller of inject_virtual_interrupt() ignores the return value
anyway. This allows us to simplify eventfd_signal() in follow-up
patches.

Signed-off-by: Christian Brauner <[email protected]>
---
drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
index de3f5903d1a7..9665876b4b13 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.c
+++ b/drivers/gpu/drm/i915/gvt/interrupt.c
@@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq)
#define MSI_CAP_DATA(offset) (offset + 8)
#define MSI_CAP_EN 0x1

-static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
+static void inject_virtual_interrupt(struct intel_vgpu *vgpu)
{
unsigned long offset = vgpu->gvt->device_info.msi_cap_offset;
u16 control, data;
@@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)

/* Do not generate MSI if MSIEN is disabled */
if (!(control & MSI_CAP_EN))
- return 0;
+ return;

if (WARN(control & GENMASK(15, 1), "only support one MSI format\n"))
- return -EINVAL;
+ return;

trace_inject_msi(vgpu->id, addr, data);

@@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
* returned and don't inject interrupt into guest.
*/
if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
- return -ESRCH;
- if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1)
- return -EFAULT;
- return 0;
+ return;
+ if (!vgpu->msi_trigger)
+ return;
+ eventfd_signal(vgpu->msi_trigger, 1);
}

static void propagate_event(struct intel_gvt_irq *irq,

--
2.42.0


2023-11-22 15:05:35

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] i915: make inject_virtual_interrupt() void

On Wed 22-11-23 13:48:22, Christian Brauner wrote:
> The single caller of inject_virtual_interrupt() ignores the return value
> anyway. This allows us to simplify eventfd_signal() in follow-up
> patches.
>
> Signed-off-by: Christian Brauner <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

Honza

> ---
> drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
> index de3f5903d1a7..9665876b4b13 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq)
> #define MSI_CAP_DATA(offset) (offset + 8)
> #define MSI_CAP_EN 0x1
>
> -static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
> +static void inject_virtual_interrupt(struct intel_vgpu *vgpu)
> {
> unsigned long offset = vgpu->gvt->device_info.msi_cap_offset;
> u16 control, data;
> @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
>
> /* Do not generate MSI if MSIEN is disabled */
> if (!(control & MSI_CAP_EN))
> - return 0;
> + return;
>
> if (WARN(control & GENMASK(15, 1), "only support one MSI format\n"))
> - return -EINVAL;
> + return;
>
> trace_inject_msi(vgpu->id, addr, data);
>
> @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
> * returned and don't inject interrupt into guest.
> */
> if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> - return -ESRCH;
> - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1)
> - return -EFAULT;
> - return 0;
> + return;
> + if (!vgpu->msi_trigger)
> + return;
> + eventfd_signal(vgpu->msi_trigger, 1);
> }
>
> static void propagate_event(struct intel_gvt_irq *irq,
>
> --
> 2.42.0
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-11-23 00:28:17

by Zhenyu Wang

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] i915: make inject_virtual_interrupt() void

On 2023.11.22 13:48:22 +0100, Christian Brauner wrote:
> The single caller of inject_virtual_interrupt() ignores the return value
> anyway. This allows us to simplify eventfd_signal() in follow-up
> patches.
>
> Signed-off-by: Christian Brauner <[email protected]>
> ---
> drivers/gpu/drm/i915/gvt/interrupt.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
> index de3f5903d1a7..9665876b4b13 100644
> --- a/drivers/gpu/drm/i915/gvt/interrupt.c
> +++ b/drivers/gpu/drm/i915/gvt/interrupt.c
> @@ -422,7 +422,7 @@ static void init_irq_map(struct intel_gvt_irq *irq)
> #define MSI_CAP_DATA(offset) (offset + 8)
> #define MSI_CAP_EN 0x1
>
> -static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
> +static void inject_virtual_interrupt(struct intel_vgpu *vgpu)
> {
> unsigned long offset = vgpu->gvt->device_info.msi_cap_offset;
> u16 control, data;
> @@ -434,10 +434,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
>
> /* Do not generate MSI if MSIEN is disabled */
> if (!(control & MSI_CAP_EN))
> - return 0;
> + return;
>
> if (WARN(control & GENMASK(15, 1), "only support one MSI format\n"))
> - return -EINVAL;
> + return;
>
> trace_inject_msi(vgpu->id, addr, data);
>
> @@ -451,10 +451,10 @@ static int inject_virtual_interrupt(struct intel_vgpu *vgpu)
> * returned and don't inject interrupt into guest.
> */
> if (!test_bit(INTEL_VGPU_STATUS_ATTACHED, vgpu->status))
> - return -ESRCH;
> - if (vgpu->msi_trigger && eventfd_signal(vgpu->msi_trigger, 1) != 1)
> - return -EFAULT;
> - return 0;
> + return;
> + if (!vgpu->msi_trigger)
> + return;
> + eventfd_signal(vgpu->msi_trigger, 1);
> }

I think it's a little simpler to write as
if (vgpu->msi_trigger)
eventfd_signal(vgpu->msi_trigger, 1);

Looks fine with me.

Reviewed-by: Zhenyu Wang <[email protected]>

Thanks!

>
> static void propagate_event(struct intel_gvt_irq *irq,
>
> --
> 2.42.0
>


Attachments:
(No filename) (2.08 kB)
signature.asc (201.00 B)
Download all attachments

2023-11-23 13:12:36

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] i915: make inject_virtual_interrupt() void

> > + if (!vgpu->msi_trigger)
> > + return;
> > + eventfd_signal(vgpu->msi_trigger, 1);
> > }
>
> I think it's a little simpler to write as
> if (vgpu->msi_trigger)
> eventfd_signal(vgpu->msi_trigger, 1);

Good point. I folded that suggestion into the patch.