qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Subject: Re: [PATCH v4 4/4] target/ppc: Add support for Radix partition-scoped translation
Date: Wed, 8 Apr 2020 09:22:44 +0200	[thread overview]
Message-ID: <8c0894e4-43e2-2914-e7d0-e24a377670fb@kaod.org> (raw)
In-Reply-To: <20200408030931.GB304335@umbus.fritz.box>

>>> -    *raddr = g_raddr;
>>> +    /*
>>> +     * Perform partition-scoped translation if !HV or HV access to
>>> +     * quadrants 1 or 2. Translates a guest real address to a host
>>> +     * real address.
>>> +     */
>>> +    if ((lpid != 0) || (!cpu->vhyp && !msr_hv)) {
>>
>> This check is too complex for my taste. Also it doesn't seem right
>> to look at lpid if the machine is pseries, even if it would happen
>> to work because pseries cannot have lpid != 0. I think we should
>> have distinct paths for powernv and pseries.
>>
>> A bit like with the following squashed in:
>>
>> =======================================
>> --- a/target/ppc/mmu-radix64.c
>> +++ b/target/ppc/mmu-radix64.c
>> @@ -489,22 +489,28 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>>          g_raddr = eaddr & R_EADDR_MASK;
>>      }
>>  
>> -    /*
>> -     * Perform partition-scoped translation if !HV or HV access to
>> -     * quadrants 1 or 2. Translates a guest real address to a host
>> -     * real address.
>> -     */
>> -    if ((lpid != 0) || (!cpu->vhyp && !msr_hv)) {
>> -        int ret = ppc_radix64_partition_scoped_xlate(cpu, rwx, eaddr, g_raddr,
>> +    if (cpu->vhyp) {
>> +        *raddr = g_raddr;
>> +    } else {
>> +        /*
>> +         * Perform partition-scoped translation if !HV or HV access to
>> +         * quadrants 1 or 2. Translates a guest real address to a host
>> +         * real address.
>> +         */
>> +        if (lpid || !msr_hv) {
>> +            int ret;
>> +
>> +            ret = ppc_radix64_partition_scoped_xlate(cpu, rwx, eaddr, g_raddr,
>>                                                       pate, raddr, &prot, &psize,
>>                                                       0, cause_excp);
>> -        if (ret) {
>> -            return ret;
>> +            if (ret) {
>> +                return ret;
>> +            }
>> +            *psizep = MIN(*psizep, psize);
>> +            *protp &= prot;
>> +        } else {
>> +            *raddr = g_raddr;
>>          }
>> -        *psizep = MIN(*psizep, psize);
>> -        *protp &= prot;
>> -    } else {
>> -        *raddr = g_raddr;
>>      }
>>  
>>      return 0;
>> =======================================
>>
>> David,
>>
>> If my comment makes sense to you, can you squash the above fix into
>> Cedric's patch ?
> 
> Yes.  I also think we shouldn't be looking at lpid for the vhyp case.
> I've applied the rest of the series to ppc-for-5.1, and folded in this
> correction as suggested.


I explored a solution with two ppc_radix64_xlate() routines, one simple 
for pseries, a second more complex for powernv but it didn't look very
good. May be it will be easier now that the first patches are merged. 

Thanks,

C. 




      reply	other threads:[~2020-04-08  7:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-03 14:00 [PATCH v4 0/4] target/ppc: Add support for Radix partition-scoped translation Cédric Le Goater
2020-04-03 14:00 ` [PATCH v4 1/4] target/ppc: Introduce ppc_radix64_xlate() for Radix tree translation Cédric Le Goater
2020-04-03 14:56   ` Greg Kurz
2020-04-08  2:57   ` David Gibson
2020-04-03 14:00 ` [PATCH v4 2/4] target/ppc: Extend ppc_radix64_check_prot() with a 'partition_scoped' bool Cédric Le Goater
2020-04-03 14:00 ` [PATCH v4 3/4] target/ppc: Rework ppc_radix64_walk_tree() for partition-scoped translation Cédric Le Goater
2020-04-03 14:00 ` [PATCH v4 4/4] target/ppc: Add support for Radix " Cédric Le Goater
2020-04-03 15:11   ` Greg Kurz
2020-04-08  3:09     ` David Gibson
2020-04-08  7:22       ` Cédric Le Goater [this message]

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=8c0894e4-43e2-2914-e7d0-e24a377670fb@kaod.org \
    --to=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=sjitindarsingh@gmail.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).