[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [QEMU-PPC] [PATCH V4 07/11] target/ppc: Don't gen an SDR1
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [QEMU-PPC] [PATCH V4 07/11] target/ppc: Don't gen an SDR1 on POWER9 and rework register creation |
Date: |
Mon, 27 Feb 2017 16:18:40 +1100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Mon, Feb 27, 2017 at 03:06:34PM +1100, Balbir Singh wrote:
> On Fri, Feb 24, 2017 at 12:05:13PM +1100, Suraj Jitindar Singh wrote:
> > POWER9 doesn't have a storage description register 1 (SDR1) which is used
> > to store the base and size of the hash table. Thus we don't need to
> > generate this register on the POWER9 cpu model. While we're here, the
> > register generation code for 970, POWER5+, POWER<7/8/9> in general is a
> > mess where we call a generic function from a model specific function which
> > then attempts to call model specific functions, so rework this for
> > readability.
> >
> > We update ppc_cpu_dump_state so that "info registers" will only display
> > the value of sdr1 if the register has been generated.
> >
> > As mentioned above the register generation for the pcc->init_proc
> > function for 970, POWER5+, POWER7, POWER8 and POWER9 has been reworked
> > for improved clarity. Instead of calling init_proc_book3s_64 which then
> > attempts to generate the correct registers through a mess of if statements,
> > we remove this function and instead call the appropriate register
> > generation functions directly. This follows the register generation model
> > used for earlier cpu models (pre-970) whereby cpu specific registers are
> > generated directly in the init_proc function and makes it easier to
> > add/remove specific registers for new cpu models.
> >
>
> Honestly I prefer less code duplication and I don't think the parsing of
> the generation based on model numbers is complex. The problem with duplication
> is it almost always leads to BUGS
Usually, I'd agree with you, but in this case I think the existing set
of helpers and conditionals is so convoluted that the duplication
makes sense.
The new duplication could well lead to bugs in the usualy way, i.e. a
fix might be made in only one place when it needs to be in several.
However, I think that's more than offset by the likelihood of the old
way leading to bugs by adding or removing something to a common helper
which _isn't_ actually common across quit all the various affected
target cpus.
--
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
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 05/11] target/ppc: Add patb_entry to sPAPRMachineState, (continued)
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 05/11] target/ppc: Add patb_entry to sPAPRMachineState, Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 06/11] target/ppc: Remove the function ppc_hash64_set_sdr1(), Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 08/11] target/ppc/POWER9: Add POWER9 mmu fault handler, Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 07/11] target/ppc: Don't gen an SDR1 on POWER9 and rework register creation, Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 09/11] target/ppc/POWER9: Add POWER9 pa-features definition, Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 10/11] target/ppc/POWER9: Add cpu_has_work function for POWER9, Suraj Jitindar Singh, 2017/02/23
- [Qemu-ppc] [QEMU-PPC] [PATCH V4 11/11] hw/ppc/spapr: Add POWER9 to pseries cpu models, Suraj Jitindar Singh, 2017/02/23