qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/4] spapr: remove irq_hint parameter


From: Greg Kurz
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()
Date: Mon, 28 May 2018 14:09:04 +0200

On Mon, 28 May 2018 11:20:36 +0200
Cédric Le Goater <address@hidden> wrote:

> On 05/28/2018 09:18 AM, Thomas Huth wrote:
> > On 28.05.2018 09:06, Cédric Le Goater wrote:  
> >> On 05/28/2018 08:17 AM, Thomas Huth wrote:  
> >>> On 25.05.2018 16:02, Greg Kurz wrote:  
> >>>> On Fri, 18 May 2018 18:44:02 +0200
> >>>> Cédric Le Goater <address@hidden> wrote:
> >>>>  
> >>>>> This IRQ number hint can possibly be used by the VIO devices if the
> >>>>> "irq" property is defined on the command line but it seems it is never
> >>>>> the case. It is not used in libvirt for instance. So, let's remove it
> >>>>> to simplify future changes.
> >>>>>  
> >>>>
> >>>> Setting an irq manually looks a bit anachronistic. I doubt anyone would
> >>>> do that nowadays, and the patch does a nice cleanup. So this looks like
> >>>> a good idea.  
> >>> [...]  
> >>>>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> >>>>> index 472dd6f33a96..cc064f64fccf 100644
> >>>>> --- a/hw/ppc/spapr_vio.c
> >>>>> +++ b/hw/ppc/spapr_vio.c
> >>>>> @@ -455,7 +455,7 @@ static void spapr_vio_busdev_realize(DeviceState 
> >>>>> *qdev, Error **errp)
> >>>>>          dev->qdev.id = id;
> >>>>>      }
> >>>>>  
> >>>>> -    dev->irq = spapr_irq_alloc(spapr, dev->irq, false, &local_err);
> >>>>> +    dev->irq = spapr_irq_alloc(spapr, false, &local_err);  
> >>>>
> >>>> Silently breaking "irq" like this looks wrong. I'd rather officially 
> >>>> remove
> >>>> it first (ie, kill spapr_vio_props, -5 lines in spapr_vio.c).
> >>>>
> >>>> Of course, this raises the question of interface deprecation, and it 
> >>>> should
> >>>> theoretically follow the process described at:
> >>>>
> >>>> https://wiki.qemu.org/Features/LegacyRemoval#Rules_for_removing_an_interface
> >>>>
> >>>> Cc'ing Thomas, our Chief Deprecation Officer, for insights :)  
> >>>
> >>> The property is a public interface. Just because it's not used by
> >>> libvirt does not mean that nobody is using it. So yes, please follow the
> >>> rules and mark it as deprecated first for two release, before you really
> >>> remove it.  
> >>
> >> This "irq" property is a problem to introduce a new static layout of IRQ 
> >> numbers. It is in complete opposition. 
> >>
> >> Can we keep it as it is for old pseries machine (settable) and ignore it 
> >> for newer ? Would that be fine ?  
> > 
> > I think that would be fine, too. You likely need to keep the settable
> > IRQs around for the old machines anyway, to make sure that migration of
> > the old machine types still works, right?  
> 
> Yes, that is the goal of patch 3. It introduces a common sPAPR IRQ frontend,
> the first backend being the current one.
> 

If we keep "irq" but we ignore it with newer machine types, we should at
least print a warning then IMHO.

> C.
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]