All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Daniel Henrique Barboza <danielhb413@gmail.com>
Cc: "Cédric Le Goater" <clg@kaod.org>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-devel@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	qemu-ppc@nongnu.org, kvm@vger.kernel.org,
	"Alexey Kardashevskiy" <aik@ozlabs.ru>,
	"Greg Kurz" <groug@kaod.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>
Subject: Re: [PATCH-for-8.0 4/4] hw/ppc/spapr_ovec: Avoid target_ulong spapr_ovec_parse_vector()
Date: Thu, 22 Dec 2022 12:57:44 +1100	[thread overview]
Message-ID: <Y6O5mL60bGnwiHgO@yekko> (raw)
In-Reply-To: <7c67f0e8-f7b7-8d0d-ba72-06cd2c8d29d3@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]

On Wed, Dec 21, 2022 at 10:26:51AM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 12/21/22 06:46, Cédric Le Goater wrote:
> > On 12/16/22 17:47, Daniel Henrique Barboza wrote:
> > > 
> > > 
> > > On 12/13/22 09:35, Philippe Mathieu-Daudé wrote:
> > > > spapr_ovec.c is a device, but it uses target_ulong which is
> > > > target specific. The hwaddr type (declared in "exec/hwaddr.h")
> > > > better fits hardware addresses.
> > > 
> > > As said by Harsh, spapr_ovec is in fact a data structure that stores platform
> > > options that are supported by the guest.
> > > 
> > > That doesn't mean that I oppose the change made here. Aside from semantics - which
> > > I also don't have a strong opinion about it - I don't believe it matters that
> > > much - spapr is 64 bit only, so hwaddr will always be == target_ulong.
> > > 
> > > Cedric/David/Greg, let me know if you have any restriction/thoughts about this.
> > > I'm inclined to accept it as is.
> > 
> > Well, I am not sure.
> > 
> > The vector table variable is the result of a ppc64_phys_to_real() conversion
> > of the CAS hcall parameter, which is a target_ulong, but ppc64_phys_to_real()
> > returns a uint64_t.
> > 
> > The code is not consistent in another places :
> > 
> >    hw/ppc/spapr_tpm_proxy.c uses a uint64_t
> >    hw/ppc/spapr_hcall.c, a target_ulong
> >    hw/ppc/spapr_rtas.c, a hwaddr
> >    hw/ppc/spapr_drc.c, a hwaddr indirectly
> > 
> > Should we change ppc64_phys_to_real() to return an hwaddr (also) ?
> 
> It makes sense to me a function called ppc64_phys_to_real() returning
> a hwaddr type.

Yes, I also think that makes sense.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-12-22  2:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-13 12:35 [PATCH-for-8.0 0/4] ppc: Clean up few headers to make them target agnostic Philippe Mathieu-Daudé
2022-12-13 12:35 ` [PATCH-for-8.0 1/4] target/ppc/kvm: Add missing "cpu.h" and "exec/hwaddr.h" Philippe Mathieu-Daudé
2022-12-16 16:33   ` Daniel Henrique Barboza
2022-12-13 12:35 ` [PATCH-for-8.0 2/4] hw/ppc/vof: Do not include the full "cpu.h" Philippe Mathieu-Daudé
2022-12-16 16:33   ` Daniel Henrique Barboza
2022-12-13 12:35 ` [PATCH-for-8.0 3/4] hw/ppc/spapr: Reduce "vof.h" inclusion Philippe Mathieu-Daudé
2022-12-16 16:34   ` Daniel Henrique Barboza
2022-12-13 12:35 ` [PATCH-for-8.0 4/4] hw/ppc/spapr_ovec: Avoid target_ulong spapr_ovec_parse_vector() Philippe Mathieu-Daudé
2022-12-13 16:40   ` Harsh Prateek Bora
2022-12-16 16:47   ` Daniel Henrique Barboza
2022-12-21  9:46     ` Cédric Le Goater
2022-12-21 13:26       ` Daniel Henrique Barboza
2022-12-22  1:57         ` David Gibson [this message]
2022-12-16 16:54 ` [PATCH-for-8.0 0/4] ppc: Clean up few headers to make them target agnostic Daniel Henrique Barboza
2022-12-16 20:32   ` Philippe Mathieu-Daudé

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=Y6O5mL60bGnwiHgO@yekko \
    --to=david@gibson.dropbear.id.au \
    --cc=aik@ozlabs.ru \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --cc=groug@kaod.org \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.