|
From: Florian K. <br...@ac...> - 2012-09-24 12:22:06
|
On 09/24/2012 05:55 AM, Julian Seward wrote:
>
>> /* The various kinds of caches */
>> typedef enum {
>> DATA_CACHE,
>> INSN_CACHE,
>> DATA_INSN_CACHE // combined data and insn cache
>> } cache_kind;
>
> Like JosefW I would prefer COMBINED_CACHE to DATA_INSN_CACHE.
Josef did suggest UNIFIED_CACHE as it is the standard term. I think I've
seen "unified cache" more often that "combined cache" so I'm going to
run with that unless you object.
>> /* Information about the cache system as a whole */
>> typedef struct {
>> UInt num_levels;
>> UInt num_caches;
>> /* Unordered array of caches for this host. NULL if there are
>> no caches. Users can assume that the array contains at most one
>> cache of a given kind per cache level. */
>> cache_t *caches;
>> } cacheinfo_t;
>
> IIUC, num_levels is the number of levels the machine really has,
Yes
> and
> num_caches <= num_levels is the number of caches for which we have
> descriptions. Which may be less than the number of levels. Yes?
Yes, num_caches is the number of caches for which we have descriptions.
But it can be >= num_levels. Think of a machine that has L1 data and L1
insn cache and L2 data and L2 insn cache. So num_caches == 4 and
num_levels == 2.
> Am I right to understand (from your comments later) that some s390s
> have icaches which are coherent, and some do not? So you need to know
> the model number in order to decide whether or not VG_(invalid_icache)
> is a no-op?
No that is not the case for s390. But it might be for some architecture.
I was trying to propose something to cover the general case.
>> (4) VEX: functions returning VexInvalRange
>> The returned address range indicates whether some cache invalidation
>> needs to occur later. What is returned here may, in general, depend
>> on the particular machine model of a given architecture. So we need
>> to query the cache info before returning anything.
>> A different possibility, which may even be cleaner, is to make these
>> functions *always* return the address range for the insns that were
>> patched. In that case we would not need cache information here.
>
> Yes. I would prefer this solution. The patchers just return the address
> range they patched, and it is the call site's problem to figure out what to
> do about cache coherence.
Excellent.
>> Tools can call the existing VG_(machine_get_VexArchInfo) to get the
>> cache information. The function would have to be exposed through
>> pub_tool_machine.h so it becomes available.
>> Alternatively, VexArchInfo could be passed to the "instrument" function
>> of the tools.
>
> I would prefer both, in fact -- pass it to the instrument function, and
> allow tools to make ad-hoc calls to get it, if they want.
OK. I can see how it might be convenient to have a functional interface.
Might eliminate the need for a global variable in the tools.
> A couple of other comments:
>
> * pls can we use type names beginning w/ capitals, as in the rest
> of the code base
Sure, will do.
>
> * another piece of complexity to be aware of (a small one, though) is
> that ARM sets its hwcaps not only by the normal game of trying insns
> and seeing which get SIGILL (in m_machine.c), but also by looking at
> the AUXV entries on the stack at startup.
I think ppc does this, too.
Florian
|