qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Aleksandar Markovic <aleksandar.m.mail@gmail.com>,
	BALATON Zoltan <balaton@eik.bme.hu>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>, "Jia Liu" <proljc@gmail.com>,
	"Max Filippov" <jcmvbkbc@gmail.com>,
	"Thomas Huth" <huth@tuxfamily.org>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Fabien Chouteau" <chouteau@adacore.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Aleksandar Markovic" <aleksandar.markovic@rt-rk.com>,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"KONRAD Frederic" <frederic.konrad@adacore.com>,
	"Michael Rolnik" <mrolnik@gmail.com>,
	"Michael Walle" <michael@walle.cc>,
	"Edgar E . Iglesias" <edgar.iglesias@gmail.com>,
	"Aleksandar Rikalo" <aleksandar.rikalo@rt-rk.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Richard Henderson" <rth@twiddle.net>,
	"Artyom Tarasenko" <atar4qemu@gmail.com>,
	"Laurent Vivier" <laurent@vivier.eu>
Subject: Re: [PATCH rc3 23/30] hw/core/loader: Let load_elf populate the processor-specific flags
Date: Wed, 29 Jan 2020 08:32:26 +0100	[thread overview]
Message-ID: <31a56bec-bc6c-7f05-5b96-27bde202c105@redhat.com> (raw)
In-Reply-To: <CAL1e-=i_NPErr=9Bag_6Ps1+5=0xAC0NKxpTnyCJVpvZ_a5veQ@mail.gmail.com>

Hi Aleksandar,

On 1/28/20 8:25 PM, Aleksandar Markovic wrote:
> On Tue, Jan 28, 2020 at 2:27 PM Aleksandar Markovic 
> <aleksandar.m.mail@gmail.com <mailto:aleksandar.m.mail@gmail.com>> wrote:
> 
> 
> 
>     On Tuesday, January 28, 2020, BALATON Zoltan <balaton@eik.bme.hu
>     <mailto:balaton@eik.bme.hu>> wrote:
> 
>         On Tue, 28 Jan 2020, Aleksandar Markovic wrote:
> 
>             On Sunday, January 26, 2020, Aleksandar Markovic <
>             aleksandar.markovic@rt-rk.com
>             <mailto:aleksandar.markovic@rt-rk.com>> wrote:
> 
>                 From: Philippe Mathieu-Daudé <f4bug@amsat.org
>                 <mailto:f4bug@amsat.org>>
> 
>                 Some platforms (like AVR) need to determine cpu type by
>                 reading
>                 the ELF flags (field e_flags oin ELF header).
> 
>                 This patch enables discovery of the content of that flag
>                 while
>                 using following functions:
> 
>                    - load_elf()
>                    - load_elf_as()
>                    - load_elf_ram()
>                    - load_elf_ram_sym()
> 
>                 The added argument of these functions is of type
>                 uint32_t*. It is
>                 allowed to pass NULL as that argument, and in such case
>                 no lookup
>                 to the field e_flags will happen, and of course, no
>                 information
>                 will be returned to the caller.
> 
> 
> Applied to MIPS queue, with some commit message corrections and fixes.

Sorry I didn't respond earlier, this was a very short delay (patch 
posted yesterday, pull request sent today).

My original patch was much less intrusive:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg673762.html
I don't find comfortable being listed as the author of the current
patch. Do you mind changing the authorship?

Thank you,

Phil.

> 
>             We plan to use this new functionality for MIPS Malta board,
>             for detection
>             of incompatible cpu/kernel combinations, and graceful exit
>             (right now these
>             combinations result in hang or crash). I would say other
>             boards could make
>             use of it too.
> 
>             For that reason, if there is no objection, I plan to select
>             this patch for
>             next MIPS queue.
> 
> 
>         No objection but kind of déjà vu:
> 
>         https://lists.nongnu.org/archive/html/qemu-devel/2019-01/msg03427.html
> 
>         I still think the interface of load_elf may need to be rethinked
>         but I don't know a good way.
> 
> 
> 
>     Perhaps having only two, "in" and "out", arguments that are pointers
>     to structures?
> 
>     Another thing that I noticed is "endian argument" that it seems
>     everyone uses in a different way: 0, 1, true, false, bigendian, etc.
>     Would c enumeration help? This looks to me like a time ticking bomb.
> 
>     Just to add that some platforms like MIPS and SPARC must load elfs
>     of more than one value of EM_MACHINE (in MIPS case, EM_MIPS and
>     EM_NANOMIPS) and current load_elf() interface offers only clumsy
>     solutions/workarounds in these cases.
> 
>     Let's think about everything later on.
> 
> 
>           This could be fixed in a later patch causing more code churn
>         again though, so if there's a way to fix this it might be a good
>         opportunity now. But I don't want to hold your patch series back
>         so unless someone has a good idea to avoid this situation then
>         we have to live with it.
> 
> 
>     Thank you. I will do some minor corrections for obvious unclarities
>     and typos in the commit message while applying to my qieue.
> 
>         Regards,
>         BALATON Zoltan
> 
> 
>             Regards,
>             Aleksandar
> 
> 
> 
> 
>                 CC: Richard Henderson <rth@twiddle.net
>                 <mailto:rth@twiddle.net>>
>                 CC: Peter Maydell <peter.maydell@linaro.org
>                 <mailto:peter.maydell@linaro.org>>
>                 CC: Edgar E. Iglesias <edgar.iglesias@gmail.com
>                 <mailto:edgar.iglesias@gmail.com>>
>                 CC: Michael Walle <michael@walle.cc>
>                 CC: Thomas Huth <huth@tuxfamily.org
>                 <mailto:huth@tuxfamily.org>>
>                 CC: Laurent Vivier <laurent@vivier.eu
>                 <mailto:laurent@vivier.eu>>
>                 CC: Philippe Mathieu-Daudé <f4bug@amsat.org
>                 <mailto:f4bug@amsat.org>>
>                 CC: Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com
>                 <mailto:aleksandar.rikalo@rt-rk.com>>
>                 CC: Aurelien Jarno <aurelien@aurel32.net
>                 <mailto:aurelien@aurel32.net>>
>                 CC: Jia Liu <proljc@gmail.com <mailto:proljc@gmail.com>>
>                 CC: David Gibson <david@gibson.dropbear.id.au
>                 <mailto:david@gibson.dropbear.id.au>>
>                 CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk
>                 <mailto:mark.cave-ayland@ilande.co.uk>>
>                 CC: BALATON Zoltan <balaton@eik.bme.hu
>                 <mailto:balaton@eik.bme.hu>>
>                 CC: Christian Borntraeger <borntraeger@de.ibm.com
>                 <mailto:borntraeger@de.ibm.com>>
>                 CC: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
>                 CC: Artyom Tarasenko <atar4qemu@gmail.com
>                 <mailto:atar4qemu@gmail.com>>
>                 CC: Fabien Chouteau <chouteau@adacore.com
>                 <mailto:chouteau@adacore.com>>
>                 CC: KONRAD Frederic <frederic.konrad@adacore.com
>                 <mailto:frederic.konrad@adacore.com>>
>                 CC: Max Filippov <jcmvbkbc@gmail.com
>                 <mailto:jcmvbkbc@gmail.com>>
> 
>                 Signed-off-by: Michael Rolnik <mrolnik@gmail.com
>                 <mailto:mrolnik@gmail.com>>
>                 Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com
>                 <mailto:amarkovic@wavecomp.com>>
>                 [PMD: Extracted from bigger patch,
>                        Replaced 'uint32_t *pe_flags' by 'int proc_flags']
>                 [AM: Replaced 'int proc_flags' with 'uint32_t *pflags',
>                       replaced one instance of 'elf_sword' to 'elf_word',
>                       extended functionality to load_elf()]
>                 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org
>                 <mailto:f4bug@amsat.org>>
>                 Signed-off-by: Aleksandar Markovic
>                 <aleksandar.m.mail@gmail.com
>                 <mailto:aleksandar.m.mail@gmail.com>>
>                 ---
>                   hw/alpha/dp264.c               |  4 ++--
>                   hw/arm/armv7m.c                |  2 +-
>                   hw/arm/boot.c                  |  2 +-
>                   hw/core/generic-loader.c       |  2 +-
>                   hw/core/loader.c               | 37
>                 +++++++++++++++++++------------------
>                   hw/cris/boot.c                 |  2 +-
>                   hw/hppa/machine.c              |  4 ++--
>                   hw/i386/multiboot.c            |  2 +-
>                   hw/i386/x86.c                  |  2 +-
>                   hw/lm32/lm32_boards.c          |  4 ++--
>                   hw/lm32/milkymist.c            |  2 +-
>                   hw/m68k/an5206.c               |  2 +-
>                   hw/m68k/mcf5208.c              |  2 +-
>                   hw/m68k/q800.c                 |  2 +-
>                   hw/microblaze/boot.c           |  4 ++--
>                   hw/mips/mips_fulong2e.c        |  2 +-
>                   hw/mips/mips_malta.c           |  3 ++-
>                   hw/mips/mips_mipssim.c         |  2 +-
>                   hw/mips/mips_r4k.c             |  2 +-
>                   hw/moxie/moxiesim.c            |  2 +-
>                   hw/nios2/boot.c                |  4 ++--
>                   hw/openrisc/openrisc_sim.c     |  2 +-
>                   hw/pci-host/prep.c             |  3 ++-
>                   hw/ppc/e500.c                  |  2 +-
>                   hw/ppc/mac_newworld.c          |  4 ++--
>                   hw/ppc/mac_oldworld.c          |  4 ++--
>                   hw/ppc/ppc440_bamboo.c         |  2 +-
>                   hw/ppc/sam460ex.c              |  3 ++-
>                   hw/ppc/spapr.c                 |  6 +++---
>                   hw/ppc/virtex_ml507.c          |  2 +-
>                   hw/riscv/boot.c                |  4 ++--
>                   hw/s390x/ipl.c                 |  7 ++++---
>                   hw/sparc/leon3.c               |  2 +-
>                   hw/sparc/sun4m.c               |  4 ++--
>                   hw/sparc64/sun4u.c             |  5 +++--
>                   hw/tricore/tricore_testboard.c |  2 +-
>                   hw/xtensa/sim.c                |  2 +-
>                   hw/xtensa/xtfpga.c             |  2 +-
>                   include/hw/elf_ops.h           |  6 +++++-
>                   include/hw/loader.h            | 21 ++++++++++++---------
>                   40 files changed, 92 insertions(+), 79 deletions(-)



  reply	other threads:[~2020-01-29  7:33 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-26 22:54 [PATCH rc3 00/30] target/avr merger Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 01/30] target/avr: Add basic parameters for new AVR platform Aleksandar Markovic
2020-01-27  8:54   ` Michael Rolnik
2020-01-28 13:25     ` Michael Rolnik
2020-01-28 13:49       ` Aleksandar Markovic
2020-01-28 15:10         ` Michael Rolnik
2020-01-28 18:01           ` Aleksandar Markovic
2020-01-29  7:38             ` Philippe Mathieu-Daudé
2020-01-29 10:09               ` Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 02/30] target/avr: Introduce AVR CPU class object Aleksandar Markovic
2020-01-27  2:39   ` Aleksandar Markovic
2020-01-27  8:03     ` Philippe Mathieu-Daudé
2020-01-27  8:43       ` Aleksandar Markovic
2020-01-27  8:51     ` Michael Rolnik
2020-01-29 12:20   ` Sarah Harris
2020-01-29 15:11     ` Aleksandar Markovic
2020-01-29 15:34       ` Michael Rolnik
2020-01-26 22:54 ` [PATCH rc3 03/30] target/avr: Add migration support Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 04/30] target/avr: Add GDB support Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 05/30] target/avr: Introduce AVR features Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 06/30] target/avr: Add defintions of AVR core types Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 07/30] target/avr: Add instruction helpers Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 08/30] target/avr: Add instruction translation - Registers definition Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 09/30] target/avr: Add instruction translation - Arithmetic and Logic Instructions Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 10/30] target/avr: Add instruction translation - Branch Instructions Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 11/30] target/avr: Add instruction translation - Data Transfer Instructions Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 12/30] target/avr: Add instruction translation - Bit and Bit-test Instructions Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 13/30] target/avr: Add instruction translation - MCU Control Instructions Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 14/30] target/avr: Add instruction translation - CPU main translation function Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 15/30] target/avr: Add instruction disassembly function Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 16/30] hw/char: Add limited support for Atmel USART peripheral Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 17/30] hw/timer: Add limited support for Atmel 16 bit timer peripheral Aleksandar Markovic
2020-01-26 22:54 ` [PATCH rc3 18/30] hw/misc: Add Atmel power device Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 19/30] target/avr: Add section about AVR into QEMU documentation Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 20/30] target/avr: Register AVR support with the rest of QEMU Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 21/30] target/avr: Add machine none test Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 22/30] target/avr: Update MAINTAINERS file Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 23/30] hw/core/loader: Let load_elf populate the processor-specific flags Aleksandar Markovic
2020-01-27  8:05   ` Philippe Mathieu-Daudé
2020-01-28 10:01   ` Aleksandar Markovic
2020-01-28 12:42     ` BALATON Zoltan
2020-01-28 13:27       ` Aleksandar Markovic
2020-01-28 19:25         ` Aleksandar Markovic
2020-01-29  7:32           ` Philippe Mathieu-Daudé [this message]
2020-01-29 10:02             ` Aleksandar Markovic
2020-01-29 13:56   ` Aleksandar Rikalo
2020-01-26 22:55 ` [PATCH rc3 24/30] hw/avr: Add helper to load raw/ELF firmware binaries Aleksandar Markovic
2020-01-27  7:25   ` Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 25/30] hw/avr: Add some ATmega microcontrollers Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 26/30] hw/avr: Add some Arduino boards Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 27/30] target/avr: Update build system Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 28/30] tests/boot-serial-test: Test some Arduino boards (AVR based) Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 29/30] tests/acceptance: Test the Arduino MEGA2560 board Aleksandar Markovic
2020-01-26 22:55 ` [PATCH rc3 30/30] .travis.yml: Run the AVR acceptance tests Aleksandar Markovic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=31a56bec-bc6c-7f05-5b96-27bde202c105@redhat.com \
    --to=philmd@redhat.com \
    --cc=aleksandar.m.mail@gmail.com \
    --cc=aleksandar.markovic@rt-rk.com \
    --cc=aleksandar.rikalo@rt-rk.com \
    --cc=atar4qemu@gmail.com \
    --cc=aurelien@aurel32.net \
    --cc=balaton@eik.bme.hu \
    --cc=borntraeger@de.ibm.com \
    --cc=chouteau@adacore.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=edgar.iglesias@gmail.com \
    --cc=f4bug@amsat.org \
    --cc=frederic.konrad@adacore.com \
    --cc=huth@tuxfamily.org \
    --cc=jcmvbkbc@gmail.com \
    --cc=laurent@vivier.eu \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=michael@walle.cc \
    --cc=mrolnik@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=proljc@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).