Re: [Qemu-devel] [PATCH v3 7/9] add L2x0/PL310 cache controller device

[Peter Maydell]

On 27 December 2011 20:13, Mark Langsdorf <address@hidden> wrote:
> +#define DEFAULT_CACHE_TYPE 0x19080800

Could use a comment saying what this actually is.
As far as I can tell it's actually specifying an invalid
I/D size for a PL310, which makes me suspicious of it.
(I might have misdecoded the bit fields, do check; also see below)

> +static void l2x0_priv_reset(DeviceState *dev)
> +{
> +    l2x0_state *s = DO_UPCAST(l2x0_state, busdev.qdev, dev);
> +
> +    s->cache_type = 0x1c100100;

cache_type is a constant (qdev property), right? You shouldn't
need to touch it in the reset function...

Also this number (which matches the PL310 TRM reset value)
doesn't match your DEFAULT_CACHE_TYPE constant. I know which
I'd rather make the default :-)

I've just looked a bit more closely at the TRM, and many
of the bits in the cache_type actually track bits from
the auxcr. Specifically:
bits [22:20], [10:8] track auxcr [19:17]
bits [18], [6] track auxcr [16]
(on PL220 [18:15], [6:3] track auxcr [16:13], which is an
awkward distinction between the two although I don't suppose
it's one anybody's taking advantage of.)

(the remaining bits in cache_type are for banking, lockdown
and harvard-vs-unified cache.)

-- PMM