Re: [Qemu-devel] [RFC PATCH 0/2] target-ppc migration fixes

[da...@gibson.dropbear.id.au]

On Sun, Sep 20, 2015 at 10:31:01PM +0200, Alexander Graf wrote:
> 
> 
> On 14.09.15 21:30, Mark Cave-Ayland wrote:
> > Whilst trying to fix migration of g3beige/mac99 images I came up with the
> > following patchset. The first patch is really cosmetic, while the second 
> > patch
> > alters the migration stream to include internal CPU IRQ state which appears
> > to fix an issue where images randomly fail to resume after migration.
> > 
> > As the second patch would need more work if deemed correct (the change in 
> > migration stream would require a bump in version number), it seemed worth
> > putting this out for review in case this is actually the symptom of another
> > bug.
> > 
> > Signed-off-by: Mark Cave-Ayland 
> 
> David, when a non-RFC version of this patch comes around, could you
> please review and if good apply it to the tree via your branch?
> 
> Thanks a bunch!

Mark,

I haven't seen a revised version of this.  Is that because it hasn't
been posted, or just because I've missed it somehow?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size

[da...@gibson.dropbear.id.au]

On Fri, Jun 17, 2016 at 10:55:47PM +, Scott Wood wrote:
> On 06/17/2016 05:13 PM, Aaron Larson wrote:
> > When e500 PPC is booted multi-core, the non-boot cores are started via
> > the spin table.  ppce500_spin.c:spin_kick() calls
> > mmubooke_create_initial_mapping() to allocate a 64MB TLB entry, but
> > the created TLB entry is only 256KB.
> > 
> > The root cause is that the function computing the size of the TLB
> > entry, namely booke206_page_size_to_tlb assumes MAS1.TSIZE as defined
> > by latter PPC cores, specifically (n**4)KB. The result is then used by
> > mmubooke_create_initial_mapping using MAS1_TSIZE_SHIFT, but
> > MAS1_TSIZE_SHIFT is defined assuming TLB entries are (n**2)KB. I.e., a
> > difference of shift=7 or shift=8.
> > 
> > Simply changing MAS1_TSIZE_SHIFT from 7 to 8 is not appropriate since
> > the macro is used elsewhere.
> > 
> > Signed-off-by: Aaron Larson 
> > ---
> >  hw/ppc/ppce500_spin.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
> > index 76bd78b..7e38f0c 100644
> > --- a/hw/ppc/ppce500_spin.c
> > +++ b/hw/ppc/ppce500_spin.c
> > @@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
> >  /* Create -kernel TLB entries for BookE, linearly spanning 256MB.  */
> >  static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
> >  {
> > -return ctz32(size >> 10) >> 1;
> > +/* The EREF indicates that TLB pages are (4 to the power of 2)KB, which
> > + * corresponds to MAS1_TSIZE_SHIFT=8, but to support legacy processors 
> > that
> > + * assume TLB pages are (2 to the power of 2)KB MAS1_TSIZE_SHIFT is
> > + * currently 7. */
> 
> This is backwards. It's the old processors that can only handle
> power-of-4 sizes.

To clarify, is this a problem in the code, or just in the comment?

> > +return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
> 
> The patch that changed MAS1_TSIZE_SHIFT from 8 to 7 was around the same
> time as the patch that added this code, which is probably why adjusting
> it got missed.  Commit 2bd9543cd3 did update the equivalent code in
> ppce500_mpc8544ds.c, which now resides in hw/ppc/e500.c and has been
> changed to not assume a power-of-2 size.  The ppce500_spin version
> should be eliminated.

Sounds sensible.

Aaron, for some reason I got multiple copies of your patch mail - a
couple of full ones and then a couple more extras which had 0 size.
Was that just something going wrong with your mailer, or did you
attempt to send a couple of different versions?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH] ppc: remove excessive logging

[da...@gibson.dropbear.id.au]

On Mon, Dec 16, 2019 at 09:27:13AM +0100, Thomas Huth wrote:
> On 15/12/2019 22.15, Joakim Tjernlund wrote:
> [...]
> >> LOG_EXCP() is not enabled by default, you have to edit source to enable it
> > 
> > LOG_EXCP is enabled on Gentoo, what about other distros?
> 
> I don't think that this is enabled by any other distro. Why is this
> enabled on Gentoo at all? It really should not be enabled in builds that
> are supposed to be used by normal users. Have you tried to contact the
> package maintainers of the QEMU Gentoo package and asked them to disable
> it there again?

I concur.  LOG_EXCP is definitely there for qemu developer debugging,
it's not intended for use in "normal" builds.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree

[da...@gibson.dropbear.id.au]

On Thu, Oct 14, 2021 at 05:02:59PM +, Luis Fernando Fujita Pires wrote:
> Ping?

I'm not sure who you're asking for what.  From my PoV, I'm waiting for reviews.

> 
> > -Original Message-
> > From: Luis Fernando Fujita Pires 
> > Sent: segunda-feira, 20 de setembro de 2021 15:51
> > To: Luis Fernando Fujita Pires ; qemu-
> > de...@nongnu.org; qemu-...@nongnu.org
> > Cc: da...@gibson.dropbear.id.au; gr...@kaod.org;
> > richard.hender...@linaro.org
> > Subject: RE: [PATCH v3 00/22] target/ppc: DFP instructions using decodetree
> > 
> > Ping.
> > 
> > Patches 1-4 were already applied, and patches 5-8, 12, 15, 18 are missing
> > reviews.
> > 
> > Thanks,
> > 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] Add PowerPC 32-bit guest memory dump support

[da...@gibson.dropbear.id.au]

On Wed, Feb 08, 2017 at 08:39:36PM +, Nawrocki, Michael wrote:
> This patch extends support for the `dump-guest-memory` command to the 32-bit 
> PowerPC architecture. It relies on the assumption that a 64-bit guest will 
> not dump a 32-bit core file (and vice versa); if this assumption is invalid, 
> please let me know.
> 
> Signed-off-by: Mike Nawrocki 

Sorry I've taken so long to review this.  I'm not really sure if this
is correct for all machine type combinations (particularly 32-bit
machines with a 64-bit compiled qemu), but fairly clearly it does
strictly more than the existing code.

Unfortunately, updates since you posted this means it no longer
applies, so you'll need to rebase.

When I tried applying, git am also had some complaints about trailing
whitespace.

> ---
>  target/ppc/Makefile.objs|   4 +-
>  target/ppc/arch_dump.c  | 154 
> 
>  target/ppc/cpu.h|   2 +
>  target/ppc/translate_init.c |   5 +-
>  4 files changed, 91 insertions(+), 74 deletions(-)
> 
> diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
> index a8c7a30..f50ffac 100644
> --- a/target/ppc/Makefile.objs
> +++ b/target/ppc/Makefile.objs
> @@ -1,8 +1,8 @@
>  obj-y += cpu-models.o
>  obj-y += translate.o
>  ifeq ($(CONFIG_SOFTMMU),y)
> -obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
> -obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o compat.o
> +obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o arch_dump.o
> +obj-$(TARGET_PPC64) += mmu-hash64.o compat.o
>  endif
>  obj-$(CONFIG_KVM) += kvm.o
>  obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 40282a1..28d9cc7 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -1,5 +1,5 @@
>  /*
> - * writing ELF notes for ppc64 arch
> + * writing ELF notes for ppc{64,} arch
>   *
>   *
>   * Copyright IBM, Corp. 2013
> @@ -19,36 +19,48 @@
>  #include "sysemu/dump.h"
>  #include "sysemu/kvm.h"
>  
> -struct PPC64UserRegStruct {
> -uint64_t gpr[32];
> -uint64_t nip;
> -uint64_t msr;
> -uint64_t orig_gpr3;
> -uint64_t ctr;
> -uint64_t link;
> -uint64_t xer;
> -uint64_t ccr;
> -uint64_t softe;
> -uint64_t trap;
> -uint64_t dar;
> -uint64_t dsisr;
> -uint64_t result;
> +#ifdef TARGET_PPC64
> +#define ELFCLASS ELFCLASS64
> +#define cpu_to_dump_reg cpu_to_dump64
> +typedef uint64_t reg_t;
> +typedef Elf64_Nhdr Elf_Nhdr;
> +#else
> +#define ELFCLASS ELFCLASS32
> +#define cpu_to_dump_reg cpu_to_dump32
> +typedef uint32_t reg_t;
> +typedef Elf32_Nhdr Elf_Nhdr;
> +#endif /* TARGET_PPC64 */
> +
> +struct PPCUserRegStruct {
> +reg_t gpr[32];
> +reg_t nip;
> +reg_t msr;
> +reg_t orig_gpr3;
> +reg_t ctr;
> +reg_t link;
> +reg_t xer;
> +reg_t ccr;
> +reg_t softe;
> +reg_t trap;
> +reg_t dar;
> +reg_t dsisr;
> +reg_t result;
>  } QEMU_PACKED;
>  
> -struct PPC64ElfPrstatus {
> +struct PPCElfPrstatus {
>  char pad1[112];
> -struct PPC64UserRegStruct pr_reg;
> -uint64_t pad2[4];
> +struct PPCUserRegStruct pr_reg;
> +reg_t pad2[4];
>  } QEMU_PACKED;
>  
>  
> -struct PPC64ElfFpregset {
> +struct PPCElfFpregset {
>  uint64_t fpr[32];
> -uint64_t fpscr;
> +reg_t fpscr;
>  }  QEMU_PACKED;
>  
>  
> -struct PPC64ElfVmxregset {
> +struct PPCElfVmxregset {
>  ppc_avr_t avr[32];
>  ppc_avr_t vscr;
>  union {
> @@ -57,26 +69,26 @@ struct PPC64ElfVmxregset {
>  } vrsave;
>  }  QEMU_PACKED;
>  
> -struct PPC64ElfVsxregset {
> +struct PPCElfVsxregset {
>  uint64_t vsr[32];
>  }  QEMU_PACKED;
>  
> -struct PPC64ElfSperegset {
> +struct PPCElfSperegset {
>  uint32_t evr[32];
>  uint64_t spe_acc;
>  uint32_t spe_fscr;
>  }  QEMU_PACKED;
>  
>  typedef struct noteStruct {
> -Elf64_Nhdr hdr;
> +Elf_Nhdr hdr;
>  char name[5];
>  char pad3[3];
>  union {
> -struct PPC64ElfPrstatus  prstatus;
> -struct PPC64ElfFpregset  fpregset;
> -struct PPC64ElfVmxregset vmxregset;
> -struct PPC64ElfVsxregset vsxregset;
> -struct PPC64ElfSperegset speregset;
> +struct PPCElfPrstatus  prstatus;
> +struct PPCElfFpregset  fpregset;
> +struct PPCElfVmxregset vmxregset;
> +struct PPCElfVsxregset vsxregset;
> +struct PPCElfSperegset speregset;
>  } contents;
>  } QEMU_PACKED Note;
>  
> @@ -85,12 +97,12 @@ typedef struct NoteFuncArg {
>  DumpState *state;
>  } NoteFuncArg;
>  
> -static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>  {
>  int i;
> -uint64_t cr;
> -struct PPC64ElfPrstatus *prstatus;
> -struct PPC64UserRegStruct *reg;
> +reg_t cr;
> +struct PPCElfPrstatus *prstatus;
> +struct PPCUserRegStruct *reg;
>  Note *note = &arg->note;
>  DumpState *s = arg->state;

Re: [Qemu-devel] [PATCH] Add PowerPC 32-bit guest memory dump support

[da...@gibson.dropbear.id.au]

On Wed, Feb 08, 2017 at 08:39:36PM +, Nawrocki, Michael wrote:

> This patch extends support for the `dump-guest-memory` command to
> the 32-bit PowerPC architecture. It relies on the assumption that a
> 64-bit guest will not dump a 32-bit core file (and vice versa); if
> this assumption is invalid, please let me know.

Erm..  I'm trying to work out exactly what you mean by that
assumption.

A TARGET_PPC64=y qemu supports 32-bit as well as 64-bit machine types,
so it could be running a 32-bit guest inside it.

So, arguably compile time switching between 32bit and 64-bit dumps
isn't quite correct; but then it's not any worse than what we have now
which is a compile time switch between 64-bit dumps and nothing at
all.

I think this change should be ok, but I'm no ELF or dump expert.

> Signed-off-by: Mike Nawrocki 
> ---
>  target/ppc/Makefile.objs|   4 +-
>  target/ppc/arch_dump.c  | 154 
> 
>  target/ppc/cpu.h|   2 +
>  target/ppc/translate_init.c |   5 +-
>  4 files changed, 91 insertions(+), 74 deletions(-)
> 
> diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
> index a8c7a30..f50ffac 100644
> --- a/target/ppc/Makefile.objs
> +++ b/target/ppc/Makefile.objs
> @@ -1,8 +1,8 @@
>  obj-y += cpu-models.o
>  obj-y += translate.o
>  ifeq ($(CONFIG_SOFTMMU),y)
> -obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
> -obj-$(TARGET_PPC64) += mmu-hash64.o arch_dump.o compat.o
> +obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o arch_dump.o
> +obj-$(TARGET_PPC64) += mmu-hash64.o compat.o
>  endif
>  obj-$(CONFIG_KVM) += kvm.o
>  obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
> diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
> index 40282a1..28d9cc7 100644
> --- a/target/ppc/arch_dump.c
> +++ b/target/ppc/arch_dump.c
> @@ -1,5 +1,5 @@
>  /*
> - * writing ELF notes for ppc64 arch
> + * writing ELF notes for ppc{64,} arch
>   *
>   *
>   * Copyright IBM, Corp. 2013
> @@ -19,36 +19,48 @@
>  #include "sysemu/dump.h"
>  #include "sysemu/kvm.h"
>  
> -struct PPC64UserRegStruct {
> -uint64_t gpr[32];
> -uint64_t nip;
> -uint64_t msr;
> -uint64_t orig_gpr3;
> -uint64_t ctr;
> -uint64_t link;
> -uint64_t xer;
> -uint64_t ccr;
> -uint64_t softe;
> -uint64_t trap;
> -uint64_t dar;
> -uint64_t dsisr;
> -uint64_t result;
> +#ifdef TARGET_PPC64
> +#define ELFCLASS ELFCLASS64
> +#define cpu_to_dump_reg cpu_to_dump64
> +typedef uint64_t reg_t;
> +typedef Elf64_Nhdr Elf_Nhdr;
> +#else
> +#define ELFCLASS ELFCLASS32
> +#define cpu_to_dump_reg cpu_to_dump32
> +typedef uint32_t reg_t;
> +typedef Elf32_Nhdr Elf_Nhdr;
> +#endif /* TARGET_PPC64 */
> +
> +struct PPCUserRegStruct {
> +reg_t gpr[32];
> +reg_t nip;
> +reg_t msr;
> +reg_t orig_gpr3;
> +reg_t ctr;
> +reg_t link;
> +reg_t xer;
> +reg_t ccr;
> +reg_t softe;
> +reg_t trap;
> +reg_t dar;
> +reg_t dsisr;
> +reg_t result;
>  } QEMU_PACKED;
>  
> -struct PPC64ElfPrstatus {
> +struct PPCElfPrstatus {
>  char pad1[112];
> -struct PPC64UserRegStruct pr_reg;
> -uint64_t pad2[4];
> +struct PPCUserRegStruct pr_reg;
> +reg_t pad2[4];
>  } QEMU_PACKED;
>  
>  
> -struct PPC64ElfFpregset {
> +struct PPCElfFpregset {
>  uint64_t fpr[32];
> -uint64_t fpscr;
> +reg_t fpscr;
>  }  QEMU_PACKED;
>  
>  
> -struct PPC64ElfVmxregset {
> +struct PPCElfVmxregset {
>  ppc_avr_t avr[32];
>  ppc_avr_t vscr;
>  union {
> @@ -57,26 +69,26 @@ struct PPC64ElfVmxregset {
>  } vrsave;
>  }  QEMU_PACKED;
>  
> -struct PPC64ElfVsxregset {
> +struct PPCElfVsxregset {
>  uint64_t vsr[32];
>  }  QEMU_PACKED;
>  
> -struct PPC64ElfSperegset {
> +struct PPCElfSperegset {
>  uint32_t evr[32];
>  uint64_t spe_acc;
>  uint32_t spe_fscr;
>  }  QEMU_PACKED;
>  
>  typedef struct noteStruct {
> -Elf64_Nhdr hdr;
> +Elf_Nhdr hdr;
>  char name[5];
>  char pad3[3];
>  union {
> -struct PPC64ElfPrstatus  prstatus;
> -struct PPC64ElfFpregset  fpregset;
> -struct PPC64ElfVmxregset vmxregset;
> -struct PPC64ElfVsxregset vsxregset;
> -struct PPC64ElfSperegset speregset;
> +struct PPCElfPrstatus  prstatus;
> +struct PPCElfFpregset  fpregset;
> +struct PPCElfVmxregset vmxregset;
> +struct PPCElfVsxregset vsxregset;
> +struct PPCElfSperegset speregset;
>  } contents;
>  } QEMU_PACKED Note;
>  
> @@ -85,12 +97,12 @@ typedef struct NoteFuncArg {
>  DumpState *state;
>  } NoteFuncArg;
>  
> -static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
> +static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
>  {
>  int i;
> -uint64_t cr;
> -struct PPC64ElfPrstatus *prstatus;
> -struct PPC64UserRegStruct *reg;
> +reg_t cr;
> +struct PPCElfPrstatus *prstatus;
> +struct PPCUserRegStruct *reg;
>  Note *note = &arg

Re: [PATCH] target/ppc: Fix SPE unavailable exception triggering

[da...@gibson.dropbear.id.au]

On Sun, Jul 26, 2020 at 04:59:16PM +, Matthieu Bucchianeri wrote:
> Hello Balaton,
> 
> Thank you for your thorough review! See my response below.
> 
> > > static inline void gen_evmwsmiaa(DisasContext *ctx) {
> > > -TCGv_i64 acc = tcg_temp_new_i64();
> > > -TCGv_i64 tmp = tcg_temp_new_i64();
> > > +TCGv_i64 acc;
> > > +TCGv_i64 tmp;
> > > +
> > > +if (unlikely(!ctx->spe_enabled)) {
> > > +gen_exception(ctx, POWERPC_EXCP_SPEU);
> > > +return;
> > > +}
> >
> > Isn't this missing here:
> >
> > acc = tcg_temp_new_i64();
> > tmp = tcg_temp_new_i64();
> >
> > You've removed allocating temps but this hunk does not add it back after the
> > exception unlike others in this patch.
> 
> I should have probably mentioned somewhere that this patch also
> fixes a resource leak in that function. It is not very obvious
> when looking at it as a patch, but if you take a look at the
> original code, you will see that I removed these allocations on
> purpose:
> 
>
>https://github.com/qemu/qemu/blob/d577dbaac7553767232faabb6a3e291aebd348ae/target/ppc/translate/spe-impl.inc.c#L532

Ok, can you please split the memory leak fix into a separate patch to
make this easier to review.

> 
> > static inline void gen_evmwsmiaa(DisasContext *ctx)
> > {
> > TCGv_i64 acc = tcg_temp_new_i64();
> > TCGv_i64 tmp = tcg_temp_new_i64();
> >
> > gen_evmwsmi(ctx);   /* rD := rA * rB */
> >
> > acc = tcg_temp_new_i64();
> > tmp = tcg_temp_new_i64();
> 
> I apologize for not making this any more clear in my description.
> 
> Let me know if this looks correct now, with the full context in mind.
> 
> Thanks.
> 
> LeoStella, LLC
> A joint venture of Thales Alenia Space and Spaceflight Industries
> 
> 12501 E Marginal Way S • Tukwila, WA 98168
> 
> Proprietary Document: This document may contain trade secrets or otherwise 
> proprietary and confidential information owned by LeoStella LLC. It is 
> intended only for the individual addressee and may not be copied, 
> distributed, or otherwise disclosed without LeoStella LLC express prior 
> written authorization.
> Export Controlled: This document may contain technical data whose export is 
> restricted by the Arms Export Control Act (Title 22, U.S.C., Sec 2751 et 
> seq.) or the Export Administration Act of 1979, as amended, Title 50,U.S.C., 
> app 2401 et seq. Violation of these export laws are subject to severe 
> criminal penalties. Recipient shall not export, re-export, or otherwise 
> transfer or share this document to any foreign person (as defined by U.S. 
> export laws) without advance written authorization from LeoStella LLC.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH 1/5] target/ppc: fixed GEN_OPCODE behavior when PPC_DUMP_CPU is set

[da...@gibson.dropbear.id.au]

On Wed, May 26, 2021 at 02:24:51PM -0700, Richard Henderson wrote:
> On 5/26/21 2:13 PM, Luis Fernando Fujita Pires wrote:
> > From: Bruno Larsen (billionai) 
> > > Before this patch, when PPC_DUMP_CPU is set, oname is added to
> > > opc_handler_t, but GEN_OPCODE* wouldn't set it unless DO_PPC_STATISTICS
> > > was set as well.
> > > 
> > > This patch changes it so those changes would happen when PPC_DUMP_CPU is
> > > set, but not statistics, because the latter is being removed.
> > > 
> > > Signed-off-by: Bruno Larsen (billionai) 
> > > ---
> > >   target/ppc/translate.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > I suggest removing dump_ppc_insns() altogether and 'oname' along with it.
> > 
> > Now that we're moving to decodetree, dump_ppc_insns() wouldn't show all the 
> > available opcodes anyway. And the only other locations where 'oname' is 
> > being used are when registering more than one handler for the same opcode 
> > by mistake, which won't happen anymore, as any new instructions should use 
> > decodetree.
> 
> Agreed.

I'll wait for a follow up doing this then.

> 
> r~
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [PATCH v3 4/4] target/ppc: removed all mentions to PPC_DUMP_CPU

[da...@gibson.dropbear.id.au]

On Mon, May 31, 2021 at 05:46:07PM +, Luis Fernando Fujita Pires wrote:
> From: Bruno Larsen (billionai) 
> > This feature will no longer be useful as ppc moves to using decotree for 
> > TCG.
> > And building with it enabled is no longer possible, due to changes in
> > opc_handler_t. Since the last commit that mentions it happened in 2014, I 
> > think
> > it is safe to remove it.
> > 
> > Signed-off-by: Bruno Larsen (billionai) 
> > ---
> >  target/ppc/cpu_init.c  | 205 -
> >  target/ppc/internal.h  |   2 -
> >  target/ppc/translate.c | 105 -
> >  3 files changed, 312 deletions(-)
> 
> I believe you lost Richard's review for this one. In addition to approving 
> it, he also noted the a typo in the commit message ("decotree" -> 
> "decodetree").

I folded in the typo fix and Richard's R-b for this patch and the
previous one.

> Reviewed-by: Luis Pires 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [RFC PATCH 1/4] target/ppc: move opcode table logic to translate.c

[da...@gibson.dropbear.id.au]

On Mon, Apr 26, 2021 at 07:29:54PM +, Bruno Piazera Larsen wrote:
> > > code motion to remove opcode callback table from
> > > translate_init.c.inc to translate.c in preparation
> > > to remove #include  from
> > > translate.c
> >
> > I'd mention the creation of destroy_ppc_opcodes since this patch is not
> > strictly just moving code.
> 
> Sure, will do for v2.
> 
> > > +#if defined(PPC_DUMP_CPU)
> >
> > The commented out define for this was left behind.
> 
> Good catch! The define is going to still be used by a couple of things in 
> cpu_init, though.
> I'm guessing moving to internal.h is the best solution, but correct
> me if I'm wrong

Generally LGTM, excepting the things Fabiano pointed out.

> 
> 
> Bruno Piazera Larsen
> 
> Instituto de Pesquisas 
> ELDORADO
> 
> Departamento Computação Embarcada
> 
> Analista de Software Trainee
> 
> Aviso Legal - Disclaimer

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] issue related to boot aix 7.1 when I try to use qemu ppc64

[da...@gibson.dropbear.id.au]

On Wed, Sep 11, 2019 at 03:58:14PM +, Muolo Vincenzo (S.I.) wrote:
> Hi to all
> 
> I try to use ( into VM debian 10  running into VMWARE virtualization 
> environment ) qemu ppc64  version to simulate an AIX 7.1 TL04  OS
> 
> 
> root@vkvm-acmm:/AIX# qemu-system-ppc64  -version
> QEMU emulator version 3.1.0 (Debian 1:3.1+dfsg-8+deb10u2)
> Copyright (c) 2003-2018 Fabrice Bellard and the QEMU Project developers
> 
> 
> I run the following commands into my linux DEB but after few time i have to 
> following issue ( trap  interrupt kernel see to end .. ):
> 
> qemu-system-ppc64 -cpu POWER8 -machine pseries -m 4096 -serial stdio -drive 
> file=disk.img,if=none,id=drive-virtio-disk0 -device virtio-scsi-pci,id=scsi 
> -device scsi-hd,drive=drive-virtio-disk0 -cdrom aix.iso -prom-env 
> "boot-command=dev / 0 0 s\" ibm,aix-diagnostics\" property boot 
> cdrom:\ppc\chrp\bootfile.exe -s verbose" -net nic -net tap -display vnc=:1
> W: /etc/qemu-ifup: no bridge for guest interface found

Running AIX under qemu really isn't tested or supported as yet.  We're
just starting to look at this these days, but you'd want to get the
very latest upstream qemu (4.1) and even then there could well be
remaining problems.

> 
> 
> SLOF **
> QEMU Starting
> Build Date = Dec 28 2018 13:55:34
> FW Version = buildd@ release 20180702
> Press "s" to enter Open Firmware.
> 
> Populating /vdevice methods
> Populating /vdevice/vty@7100
> Populating /vdevice/nvram@7101
> Populating /vdevice/l-lan@7102
> Populating /vdevice/v-scsi@7103
>SCSI: Looking for devices
>   8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
> Populating /pci@8002000
>  00  (D) : 1234 qemu vga
>  00 0800 (D) : 1033 0194serial bus [ usb-xhci ]
>  00 1000 (D) : 1af4 1004virtio [ scsi ]
> Populating /pci@8002000/scsi@2
>SCSI: Looking for devices
>   100 DISK : "QEMU QEMU HARDDISK2.5+"
> Installing QEMU fb
> 
> 
> 
> Scanning USB
>   XHCI: Initializing
> USB Keyboard
> USB mouse
> No console specified using screen & keyboard
> 
>   Welcome to Open Firmware
> 
>   Copyright (c) 2004, 2017 IBM Corporation All rights reserved.
>   This program and the accompanying materials are made available
>   under the terms of the BSD License available at
>   http://www.opensource.org/licenses/bsd-license.php
> 
> 
> Trying to load: -s verbose from: 
> /vdevice/v-scsi@7103/disk@8200:\ppc\chrp\bootfile.exe ...   
> Successfully loaded
> AIX
> StarLED{814}
> 
> AIX Version 7.1
> exec(/etc/init){1,0}
> 
> INIT: EXECUTING /sbin/rc.boot 1
> exec(/usr/bin/sh,-c,/sbin/rc.boot 1){1179684,1}
> exec(/sbin/rc.boot,/sbin/rc.boot,1){1179684,1}
> + PHASE=1
> + + bootinfo -p
> exec(/usr/sbin/bootinfo,-p){1245222,1179684}
> PLATFORM=chrp
> + [ ! -x /usr/lib/boot/bin/bootinfo_chrp ]
> + [ 1 -eq 1 ]
> + 1> /usr/lib/libc.a
> + init -c unlink /usr/lib/boot/bin/!(*_chrp)
> exec(/etc/init,-c,unlink /usr/lib/boot/bin/!(*_chrp)){1245224,1179684}
> + chramfs -t
> exec(/usr/sbin/chramfs,-t){1245226,1179684}
> + init -c unlink /usr/sbin/chramfs
> + 1> /dev/null
> exec(/etc/init,-c,unlink /usr/sbin/chramfs){1245228,1179684}
> + + bootinfo -t
> exec(/usr/sbin/bootinfo,-t){1245230,1179684}
> BOOTYPE=3
> + [ 0 -ne 0 ]
> + [ -z 3 ]
> + unset pdev_to_ldev undolt native_netboot_cfg
> + unset disknet_odm_init config_ATM
> + /usr/lib/methods/showled 0x510 DEV CFG 1 START
> exec(/usr/lib/methods/showled,0x510,DEV CFG 1 START){1245232,1179684}
> + cfgmgr -f -v
> exec(/usr/sbin/cfgmgr,-f,-v){1245234,1179684}
> cfgmgr is running in phase 1
> 
> Time: 0 LEDS: 0x538
> Invoking top level program -- "/etc/methods/defsys"
> exec(/bin/sh,-c,/etc/methods/defsys ){1310760,1245234}
> exec(/etc/methods/defsys){1310760,1245234}
> exec(/bin/sh,-c,/usr/lib/methods/define_rspc -n -c sys -s node -t 
> chrp){1376298,1310760}
> exec(/usr/lib/methods/define_rspc,-n,-c,sys,-s,node,-t,chrp){1376298,1310760}
> Time: 0 LEDS: 0x539
> Return code = 0
> * stdout *
> sys0
> 
> * stderr *
> MS 1376298 1310760 /usr/lib/methods/define_rspc -n -c sys -s node -t chrp
> M4 1376298 Defining device: uniquetype=sys/node/chrp parent= connection=
> M4 1376298 ..define_dvc()
> M4 1376298 ..generate_name()
> M4 1376298 Generated name: sys0
> M4 1376298 ..create_cudv()
> M4 1376298 Adding new CuDv
> M4 1376298 Defined device sys0
> 
> 
> Attempting to configure device 'sys0'
> Time: 0 LEDS: 0x811
> Invoking /usr/lib/methods/cfgsys_chrp -1 -l sys0
> exec(/bin/sh,-c,/usr/lib/methods/cfgsys_chrp -1 -l sys0){1310762,1245234}
> Number of running methods: 1
> exec(/usr/lib/methods/cfgsys_chrp,-1,-l,sys0){1310762,1245234}
> LED{A20}
> Illegal Trap Instruction Interrupt in Kernel
> 05911ACCtweqir0,0r0=0
> KDB(0)>
> 
> So how c

Re: [Qemu-devel] [RFC v1 03/18] hw/pci: introduce PCIPASIDOps to PCIDevice

[da...@gibson.dropbear.id.au]

On Wed, Jul 10, 2019 at 11:08:15AM +, Liu, Yi L wrote:
> > From: Peter Xu [mailto:zh...@redhat.com]
> > Sent: Tuesday, July 9, 2019 10:12 AM
> > To: Liu, Yi L 
> > Subject: Re: [RFC v1 03/18] hw/pci: introduce PCIPASIDOps to PCIDevice
> > 
> > On Fri, Jul 05, 2019 at 07:01:36PM +0800, Liu Yi L wrote:
> > > +void pci_setup_pasid_ops(PCIDevice *dev, PCIPASIDOps *ops)
> > > +{
> > > +assert(ops && !dev->pasid_ops);
> > > +dev->pasid_ops = ops;
> > > +}
> > > +
> > > +bool pci_device_is_ops_set(PCIBus *bus, int32_t devfn)
> > 
> > Name should be "pci_device_is_pasid_ops_set".  Or maybe you can simply
> > drop this function because as long as you check it in helper functions
> > like [1] below always then it seems even unecessary.
> 
> yes, the name should be "pci_device_is_pasid_ops_set". I noticed your
> comments on the necessity in another, let's talk in that thread. :-)
> 
> > > +{
> > > +PCIDevice *dev;
> > > +
> > > +if (!bus) {
> > > +return false;
> > > +}
> > > +
> > > +dev = bus->devices[devfn];
> > > +return !!(dev && dev->pasid_ops);
> > > +}
> > > +
> > > +int pci_device_request_pasid_alloc(PCIBus *bus, int32_t devfn,
> > > +   uint32_t min_pasid, uint32_t 
> > > max_pasid)
> > 
> > From VT-d spec I see that the virtual command "allocate pasid" does
> > not have bdf information so it's global, but here we've got bus/devfn.
> > I'm curious is that reserved for ARM or some other arch?
> 
> You are right. VT-d spec doesn’t have bdf info. But we need to pass the
> allocation request via vfio. So this function has bdf info. In vIOMMU side,
> it should select a vfio-pci device and invoke this callback when it wants to
> request PASID alloc/free.

That doesn't seem conceptually right.  IIUC, the pasids "belong" to a
sort of SVM context.  It seems to be the alloc should be on that
object - and that object would already have some connection to any
relevant vfio containers.  At the vfio level this seems like it should
be a container operation rather than a device operation.

> > > +{
> > > +PCIDevice *dev;
> > > +
> > > +if (!bus) {
> > > +return -1;
> > > +}
> > > +
> > > +dev = bus->devices[devfn];
> > > +if (dev && dev->pasid_ops && dev->pasid_ops->alloc_pasid) {
> > 
> > [1]
> > 
> > > +return dev->pasid_ops->alloc_pasid(bus, devfn, min_pasid, 
> > > max_pasid);
> 
> Thanks,
> Yi Liu

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for 4.0 v1 1/1] MAINTAINERS: Update the device tree maintainers

[da...@gibson.dropbear.id.au]

On Tue, Mar 26, 2019 at 08:59:23PM +, Alistair Francis wrote:
> Remove Alex as a Device Tree maintainer as requested by him. Add myself
> as a maintainer to avoid it being orphaned. Also add David as a
> Reviewer (R) as he is the libfdt and DTC maintainer.
> 
> Signed-off-by: Alistair Francis 

Acked-by: David Gibson 

> ---
>  MAINTAINERS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 85d7d764e5..c2ad5062f6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1813,7 +1813,8 @@ F: qom/cpu.c
>  F: include/qom/cpu.h
>  
>  Device Tree
> -M: Alexander Graf 
> +M: Alistair Francis 
> +R: David Gibson 
>  S: Maintained
>  F: device_tree.c
>  F: include/sysemu/device_tree.h

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature