Re: [Qemu-devel] [PATCHv3 05/13] sun4m_iommu: move TYPE_SUN4M_IOMMU declaration to sun4m.h

[Mark Cave-Ayland]

On 19/10/17 05:37, Philippe Mathieu-Daudé wrote:

> On 10/14/2017 03:38 PM, Mark Cave-Ayland wrote:
>> This is in preparation to allow the type to be used elsewhere.
>>
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
>>  hw/dma/sun4m_iommu.c     |   14 --------------
>>  include/hw/sparc/sun4m.h |   16 ++++++++++++++++
>>  2 files changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/hw/dma/sun4m_iommu.c b/hw/dma/sun4m_iommu.c
>> index 335ef63..840064b 100644
>> --- a/hw/dma/sun4m_iommu.c
>> +++ b/hw/dma/sun4m_iommu.c
>> @@ -36,7 +36,6 @@
>>   * 
>> http://mediacast.sun.com/users/Barton808/media/Sun4M_SystemArchitecture_edited2.pdf
>>   */
>>  
>> -#define IOMMU_NREGS         (4*4096/4)
>>  #define IOMMU_CTRL          (0x0000 >> 2)
>>  #define IOMMU_CTRL_IMPL     0xf0000000 /* Implementation */
>>  #define IOMMU_CTRL_VERS     0x0f000000 /* Version */
>> @@ -128,19 +127,6 @@
>>  #define IOMMU_PAGE_SIZE     (1 << IOMMU_PAGE_SHIFT)
>>  #define IOMMU_PAGE_MASK     ~(IOMMU_PAGE_SIZE - 1)
>>  
>> -#define TYPE_SUN4M_IOMMU "iommu"
>> -#define SUN4M_IOMMU(obj) OBJECT_CHECK(IOMMUState, (obj), TYPE_SUN4M_IOMMU)
>> -
>> -typedef struct IOMMUState {
>> -    SysBusDevice parent_obj;
>> -
>> -    MemoryRegion iomem;
>> -    uint32_t regs[IOMMU_NREGS];
>> -    hwaddr iostart;
>> -    qemu_irq irq;
>> -    uint32_t version;
>> -} IOMMUState;
>> -
>>  static uint64_t iommu_mem_read(void *opaque, hwaddr addr,
>>                                 unsigned size)
>>  {
>> diff --git a/include/hw/sparc/sun4m.h b/include/hw/sparc/sun4m.h
>> index 580d87b..1f1cf91 100644
>> --- a/include/hw/sparc/sun4m.h
>> +++ b/include/hw/sparc/sun4m.h
>> @@ -4,10 +4,26 @@
>>  #include "qemu-common.h"
>>  #include "exec/hwaddr.h"
>>  #include "qapi/qmp/types.h"
>> +#include "hw/sysbus.h"
>>  
>>  /* Devices used by sparc32 system.  */
>>  
>>  /* iommu.c */
>> +#define TYPE_SUN4M_IOMMU "iommu"
> 
> Can this be "sun4m-iommu"?

Possibly. This particular patch is concerned with moving the
declarations rather than changing them. As it's freeze coming up, I'd be
quite keen to get these changes in if possible because it nicely removes
some old sun4m hacks.

In fact, I suspect that the reason the type name is "iommu" is because
sun4m was the first IOMMU implementation within QEMU, and indeed it
predates the "official" IOMMU functionality implemented as a translating
memory region. Indeed the follow-up patchset to this switches over to
use the supported IOMMU memory region and removes another set of legacy
sun4m hacks.

>> +#define SUN4M_IOMMU(obj) OBJECT_CHECK(IOMMUState, (obj), TYPE_SUN4M_IOMMU)
>> +
>> +#define IOMMU_NREGS         (4 * 4096 / 4)
> 
> 4096?

Possibly again, same reasoning as before: this patch is just concerned
with moving the macros rather than altering them.

>> +
>> +typedef struct IOMMUState {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion iomem;
>> +    uint32_t regs[IOMMU_NREGS];
>> +    hwaddr iostart;
>> +    qemu_irq irq;
>> +    uint32_t version;
>> +} IOMMUState;
>> +
>>  void sparc_iommu_memory_rw(void *opaque, hwaddr addr,
>>                                   uint8_t *buf, int len, int is_write);
>>  static inline void sparc_iommu_memory_read(void *opaque,
> 
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>


ATB,

Mark.