Re: [PATCH 09/11] target/cpu: Restrict handlers using hwaddr type to system-mode

[Philippe Mathieu-Daudé]

On Sat, May 9, 2020 at 6:08 PM Philippe Mathieu-Daudé <address@hidden> wrote:
> On 5/9/20 3:09 PM, Philippe Mathieu-Daudé wrote:
> > Restrict the following handlers to system-mode:
> > - do_unaligned_access
> > - do_transaction_failed
> > - get_phys_page_debug
> > - get_phys_page_attrs_debug
> >
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >   include/hw/core/cpu.h           |  8 +++++---
> >   target/alpha/cpu.h              |  4 +++-
> >   target/arm/cpu.h                |  6 +++---
> >   target/arm/internals.h          |  4 ++++
> >   target/cris/cpu.h               |  2 ++
> >   target/hppa/cpu.h               |  2 +-
> >   target/i386/cpu.h               |  2 ++
> >   target/m68k/cpu.h               |  7 ++++++-
> >   target/microblaze/cpu.h         |  5 ++++-
> >   target/mips/internal.h          |  2 +-
> >   target/nios2/cpu.h              |  5 ++++-
> >   target/openrisc/cpu.h           |  3 ++-
> >   target/ppc/cpu.h                |  2 +-
> >   target/riscv/cpu.h              | 20 ++++++++++----------
> >   target/sh4/cpu.h                |  2 +-
> >   target/sparc/cpu.h              |  2 ++
> >   target/xtensa/cpu.h             | 12 +++++++-----
> >   target/hppa/cpu.c               |  4 +++-
> >   target/ppc/translate_init.inc.c |  2 +-
> >   19 files changed, 62 insertions(+), 32 deletions(-)
> >
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 5bf94d28cf..ed09d056d1 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -167,6 +167,7 @@ typedef struct CPUClass {
> >       int reset_dump_flags;
> >       bool (*has_work)(CPUState *cpu);
> >       void (*do_interrupt)(CPUState *cpu);
> > +#ifndef CONFIG_USER_ONLY
>
> I forgot once Peter Maydell told me we can't do that for some reason I
> don't remember.
>
> At least this changes the sizeof(CPUClass), so we get:
>
> qom/object.c:315:type_initialize: assertion failed: (parent->class_size
> <= ti->class_size)
>
> So we can't poison the hwaddr type? (final patch of this series).

Well, this works...:

-- >8 --
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -167,6 +167,7 @@ typedef struct CPUClass {
     int reset_dump_flags;
     bool (*has_work)(CPUState *cpu);
     void (*do_interrupt)(CPUState *cpu);
+#ifndef CONFIG_USER_ONLY
     void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
                                 MMUAccessType access_type,
                                 int mmu_idx, uintptr_t retaddr);
@@ -174,6 +175,12 @@ typedef struct CPUClass {
                                   unsigned size, MMUAccessType access_type,
                                   int mmu_idx, MemTxAttrs attrs,
                                   MemTxResult response, uintptr_t retaddr);
+    hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
+    hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
+                                        MemTxAttrs *attrs);
+#else
+    void (*reserved[4])(CPUState *cpu, ...);
+#endif /* CONFIG_USER_ONLY */
     bool (*virtio_is_big_endian)(CPUState *cpu);
     int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
                            uint8_t *buf, int len, bool is_write);
@@ -189,9 +196,6 @@ typedef struct CPUClass {
     bool (*tlb_fill)(CPUState *cpu, vaddr address, int size,
                      MMUAccessType access_type, int mmu_idx,
                      bool probe, uintptr_t retaddr);
-    hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
-    hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
-                                        MemTxAttrs *attrs);
     int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
     int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
     int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
---

Ugly?

>
> >       void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
> >                                   MMUAccessType access_type,
> >                                   int mmu_idx, uintptr_t retaddr);
> > @@ -174,6 +175,10 @@ typedef struct CPUClass {
> >                                     unsigned size, MMUAccessType 
> > access_type,
> >                                     int mmu_idx, MemTxAttrs attrs,
> >                                     MemTxResult response, uintptr_t 
> > retaddr);
> > +    hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
> > +    hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
> > +                                        MemTxAttrs *attrs);
> > +#endif /* CONFIG_USER_ONLY */
> >       bool (*virtio_is_big_endian)(CPUState *cpu);
> >       int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
> >                              uint8_t *buf, int len, bool is_write);
> > @@ -189,9 +194,6 @@ typedef struct CPUClass {
> >       bool (*tlb_fill)(CPUState *cpu, vaddr address, int size,
> >                        MMUAccessType access_type, int mmu_idx,
> >                        bool probe, uintptr_t retaddr);
> > -    hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr);
> > -    hwaddr (*get_phys_page_attrs_debug)(CPUState *cpu, vaddr addr,
> > -                                        MemTxAttrs *attrs);
> >       int (*asidx_from_attrs)(CPUState *cpu, MemTxAttrs attrs);
> >       int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
> >       int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
> [...]