[PATCH 0/7] hw/misc/empty_slot: Spring cleaning

Philippe Mathieu-Daudé posted 7 patches 3 years, 12 months ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Failed in applying to current master (apply log)
include/hw/empty_slot.h        |  9 -------
include/hw/misc/empty_slot.h   | 19 ++++++++++++++
hw/mips/mips_malta.c           |  4 +--
hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
hw/sparc/sun4m.c               | 23 +++++++++++------
MAINTAINERS                    |  4 ++-
hw/core/Makefile.objs          |  1 -
hw/misc/Makefile.objs          |  1 +
hw/misc/trace-events           |  4 +++
9 files changed, 70 insertions(+), 42 deletions(-)
delete mode 100644 include/hw/empty_slot.h
create mode 100644 include/hw/misc/empty_slot.h
rename hw/{core => misc}/empty_slot.c (66%)
[PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Philippe Mathieu-Daudé 3 years, 12 months ago
New Spring, new opportunity to clean this device :)
(v1 was in 2018, v2 in 2019).

- lower device priority
- follow qdev model and use properties
- convert to trace events
- describe with slot name
- move under hw/misc/ and cover in MAINTAINERS

Peter, I hope you are OK adding it wit UNIMP device,
as both are very similar, and don't have much activity.

Only MIPS/SPARC32 targets use this device.

v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html

Philippe Mathieu-Daudé (7):
  hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
  hw/misc/empty_slot: Lower address space priority
  hw/misc/empty_slot: Convert 'size' field as qdev property
  hw/misc/empty_slot: Add a 'name' qdev property
  hw/misc/empty_slot: Convert debug printf() to trace event
  hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
  hw/misc/empty_slot: Name the slots when created

 include/hw/empty_slot.h        |  9 -------
 include/hw/misc/empty_slot.h   | 19 ++++++++++++++
 hw/mips/mips_malta.c           |  4 +--
 hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
 hw/sparc/sun4m.c               | 23 +++++++++++------
 MAINTAINERS                    |  4 ++-
 hw/core/Makefile.objs          |  1 -
 hw/misc/Makefile.objs          |  1 +
 hw/misc/trace-events           |  4 +++
 9 files changed, 70 insertions(+), 42 deletions(-)
 delete mode 100644 include/hw/empty_slot.h
 create mode 100644 include/hw/misc/empty_slot.h
 rename hw/{core => misc}/empty_slot.c (66%)

-- 
2.21.3


Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
ping?

On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> New Spring, new opportunity to clean this device :)
> (v1 was in 2018, v2 in 2019).
> 
> - lower device priority
> - follow qdev model and use properties
> - convert to trace events
> - describe with slot name
> - move under hw/misc/ and cover in MAINTAINERS
> 
> Peter, I hope you are OK adding it wit UNIMP device,
> as both are very similar, and don't have much activity.
> 
> Only MIPS/SPARC32 targets use this device.
> 
> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> 
> Philippe Mathieu-Daudé (7):
>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>   hw/misc/empty_slot: Lower address space priority
>   hw/misc/empty_slot: Convert 'size' field as qdev property
>   hw/misc/empty_slot: Add a 'name' qdev property
>   hw/misc/empty_slot: Convert debug printf() to trace event
>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>   hw/misc/empty_slot: Name the slots when created
> 
>  include/hw/empty_slot.h        |  9 -------
>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>  hw/mips/mips_malta.c           |  4 +--
>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>  hw/sparc/sun4m.c               | 23 +++++++++++------
>  MAINTAINERS                    |  4 ++-
>  hw/core/Makefile.objs          |  1 -
>  hw/misc/Makefile.objs          |  1 +
>  hw/misc/trace-events           |  4 +++
>  9 files changed, 70 insertions(+), 42 deletions(-)
>  delete mode 100644 include/hw/empty_slot.h
>  create mode 100644 include/hw/misc/empty_slot.h
>  rename hw/{core => misc}/empty_slot.c (66%)
> 


Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Aleksandar Markovic 3 years, 11 months ago
нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
је написао/ла:
>
> ping?
>

I agree with all of your patches, they absolutely make sense to me,
but I would like to know Peter's opinion on such treatment of empty
slots.

I am going to give r-bs and integrate mips patches as soon as Peter
OKs the general approach. So, Peter, is Philippe's approach to empty
slots fine?

Aleksandar

> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> > New Spring, new opportunity to clean this device :)
> > (v1 was in 2018, v2 in 2019).
> >
> > - lower device priority
> > - follow qdev model and use properties
> > - convert to trace events
> > - describe with slot name
> > - move under hw/misc/ and cover in MAINTAINERS
> >
> > Peter, I hope you are OK adding it wit UNIMP device,
> > as both are very similar, and don't have much activity.
> >
> > Only MIPS/SPARC32 targets use this device.
> >
> > v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> > v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> >
> > Philippe Mathieu-Daudé (7):
> >   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
> >   hw/misc/empty_slot: Lower address space priority
> >   hw/misc/empty_slot: Convert 'size' field as qdev property
> >   hw/misc/empty_slot: Add a 'name' qdev property
> >   hw/misc/empty_slot: Convert debug printf() to trace event
> >   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
> >   hw/misc/empty_slot: Name the slots when created
> >
> >  include/hw/empty_slot.h        |  9 -------
> >  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
> >  hw/mips/mips_malta.c           |  4 +--
> >  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
> >  hw/sparc/sun4m.c               | 23 +++++++++++------
> >  MAINTAINERS                    |  4 ++-
> >  hw/core/Makefile.objs          |  1 -
> >  hw/misc/Makefile.objs          |  1 +
> >  hw/misc/trace-events           |  4 +++
> >  9 files changed, 70 insertions(+), 42 deletions(-)
> >  delete mode 100644 include/hw/empty_slot.h
> >  create mode 100644 include/hw/misc/empty_slot.h
> >  rename hw/{core => misc}/empty_slot.c (66%)
> >
>

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
> је написао/ла:
>>
>> ping?
>>
> 
> I agree with all of your patches, they absolutely make sense to me,
> but I would like to know Peter's opinion on such treatment of empty
> slots.
> 
> I am going to give r-bs and integrate mips patches as soon as Peter
> OKs the general approach. So, Peter, is Philippe's approach to empty
> slots fine?

Thanks Aleksandar for looking at this series.

I expect a neutral opinion from Peter.

What would be helpful is feedback from Artyom, since it authored this
device.

Artyom, do you mind Acking the series?

Thanks,

Phil.

> 
> Aleksandar
> 
>> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
>>> New Spring, new opportunity to clean this device :)
>>> (v1 was in 2018, v2 in 2019).
>>>
>>> - lower device priority
>>> - follow qdev model and use properties
>>> - convert to trace events
>>> - describe with slot name
>>> - move under hw/misc/ and cover in MAINTAINERS
>>>
>>> Peter, I hope you are OK adding it wit UNIMP device,
>>> as both are very similar, and don't have much activity.
>>>
>>> Only MIPS/SPARC32 targets use this device.
>>>
>>> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
>>> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
>>>
>>> Philippe Mathieu-Daudé (7):
>>>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>>>   hw/misc/empty_slot: Lower address space priority
>>>   hw/misc/empty_slot: Convert 'size' field as qdev property
>>>   hw/misc/empty_slot: Add a 'name' qdev property
>>>   hw/misc/empty_slot: Convert debug printf() to trace event
>>>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>>>   hw/misc/empty_slot: Name the slots when created
>>>
>>>  include/hw/empty_slot.h        |  9 -------
>>>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>>>  hw/mips/mips_malta.c           |  4 +--
>>>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>>>  hw/sparc/sun4m.c               | 23 +++++++++++------
>>>  MAINTAINERS                    |  4 ++-
>>>  hw/core/Makefile.objs          |  1 -
>>>  hw/misc/Makefile.objs          |  1 +
>>>  hw/misc/trace-events           |  4 +++
>>>  9 files changed, 70 insertions(+), 42 deletions(-)
>>>  delete mode 100644 include/hw/empty_slot.h
>>>  create mode 100644 include/hw/misc/empty_slot.h
>>>  rename hw/{core => misc}/empty_slot.c (66%)
>>>
>>
> 


Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Peter Maydell 3 years, 11 months ago
On Sun, 24 May 2020 at 21:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> > I agree with all of your patches, they absolutely make sense to me,
> > but I would like to know Peter's opinion on such treatment of empty
> > slots.
> >
> > I am going to give r-bs and integrate mips patches as soon as Peter
> > OKs the general approach. So, Peter, is Philippe's approach to empty
> > slots fine?
>
> Thanks Aleksandar for looking at this series.
>
> I expect a neutral opinion from Peter.

Yes. In particular I cannot be the bottleneck for all design
choices in QEMU: that doesn't scale. Mostly you can assume
that if I have a strong opinion then I'll provide it, and
otherwise I prefer it if the people who care enough to
maintain code and write patches to tidy it up make the choices.

(Empty slots seemed a bit odd to me last time I looked at them,
but if they work for mips and sparc that's fine with me.)

thanks
-- PMM

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Artyom Tarasenko 3 years, 11 months ago
On Sun, May 24, 2020 at 10:21 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 5/24/20 9:37 PM, Aleksandar Markovic wrote:
> > нед, 24. мај 2020. у 18:58 Philippe Mathieu-Daudé <philmd@redhat.com>
> > је написао/ла:
> >>
> >> ping?
> >>
> >
> > I agree with all of your patches, they absolutely make sense to me,
> > but I would like to know Peter's opinion on such treatment of empty
> > slots.
> >
> > I am going to give r-bs and integrate mips patches as soon as Peter
> > OKs the general approach. So, Peter, is Philippe's approach to empty
> > slots fine?
>
> Thanks Aleksandar for looking at this series.
>
> I expect a neutral opinion from Peter.
>
> What would be helpful is feedback from Artyom, since it authored this
> device.
>
> Artyom, do you mind Acking the series?

Thanks for asking and sorry for the delay. My mailer played a bad trick on me.
The patches definitely make sense:

Acked-by: Artyom Tarasenko <atar4qemu@gmail.com>



> Thanks,
>
> Phil.
>
> >
> > Aleksandar
> >
> >> On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> >>> New Spring, new opportunity to clean this device :)
> >>> (v1 was in 2018, v2 in 2019).
> >>>
> >>> - lower device priority
> >>> - follow qdev model and use properties
> >>> - convert to trace events
> >>> - describe with slot name
> >>> - move under hw/misc/ and cover in MAINTAINERS
> >>>
> >>> Peter, I hope you are OK adding it wit UNIMP device,
> >>> as both are very similar, and don't have much activity.
> >>>
> >>> Only MIPS/SPARC32 targets use this device.
> >>>
> >>> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> >>> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> >>>
> >>> Philippe Mathieu-Daudé (7):
> >>>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
> >>>   hw/misc/empty_slot: Lower address space priority
> >>>   hw/misc/empty_slot: Convert 'size' field as qdev property
> >>>   hw/misc/empty_slot: Add a 'name' qdev property
> >>>   hw/misc/empty_slot: Convert debug printf() to trace event
> >>>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
> >>>   hw/misc/empty_slot: Name the slots when created
> >>>
> >>>  include/hw/empty_slot.h        |  9 -------
> >>>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
> >>>  hw/mips/mips_malta.c           |  4 +--
> >>>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
> >>>  hw/sparc/sun4m.c               | 23 +++++++++++------
> >>>  MAINTAINERS                    |  4 ++-
> >>>  hw/core/Makefile.objs          |  1 -
> >>>  hw/misc/Makefile.objs          |  1 +
> >>>  hw/misc/trace-events           |  4 +++
> >>>  9 files changed, 70 insertions(+), 42 deletions(-)
> >>>  delete mode 100644 include/hw/empty_slot.h
> >>>  create mode 100644 include/hw/misc/empty_slot.h
> >>>  rename hw/{core => misc}/empty_slot.c (66%)
> >>>
> >>
> >
>


--
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [PATCH 0/7] hw/misc/empty_slot: Spring cleaning
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 5/10/20 5:28 PM, Philippe Mathieu-Daudé wrote:
> New Spring, new opportunity to clean this device :)
> (v1 was in 2018, v2 in 2019).
> 
> - lower device priority
> - follow qdev model and use properties
> - convert to trace events
> - describe with slot name
> - move under hw/misc/ and cover in MAINTAINERS
> 
> Peter, I hope you are OK adding it wit UNIMP device,
> as both are very similar, and don't have much activity.
> 
> Only MIPS/SPARC32 targets use this device.
> 
> v2: https://www.mail-archive.com/qemu-devel@nongnu.org/msg626498.html
> v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg564060.html
> 
> Philippe Mathieu-Daudé (7):
>   hw/sparc/sun4m: Use UnimplementedDevice for I/O devices
>   hw/misc/empty_slot: Lower address space priority
>   hw/misc/empty_slot: Convert 'size' field as qdev property
>   hw/misc/empty_slot: Add a 'name' qdev property
>   hw/misc/empty_slot: Convert debug printf() to trace event
>   hw/misc/empty_slot: Move the 'hw/misc' and cover in MAINTAINERS
>   hw/misc/empty_slot: Name the slots when created
> 
>  include/hw/empty_slot.h        |  9 -------
>  include/hw/misc/empty_slot.h   | 19 ++++++++++++++
>  hw/mips/mips_malta.c           |  4 +--
>  hw/{core => misc}/empty_slot.c | 47 +++++++++++++++++++---------------
>  hw/sparc/sun4m.c               | 23 +++++++++++------
>  MAINTAINERS                    |  4 ++-
>  hw/core/Makefile.objs          |  1 -
>  hw/misc/Makefile.objs          |  1 +
>  hw/misc/trace-events           |  4 +++
>  9 files changed, 70 insertions(+), 42 deletions(-)
>  delete mode 100644 include/hw/empty_slot.h
>  create mode 100644 include/hw/misc/empty_slot.h
>  rename hw/{core => misc}/empty_slot.c (66%)
> 

Thanks - except the MAINTAINERS change (merging empty_slot with
unimp device) which Peter did not ack - series applied to for
the next (temporary) sparc-next pull request.