Re: [Xen-devel] [PATCH v2 3/4] x86/HVM: implement memory read caching

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.09.18 at 18:20, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 11 September 2018 14:15
>> 
>> @@ -2664,9 +2685,35 @@ void hvm_dump_emulation_state(const char
>>             hvmemul_ctxt->insn_buf);
>>  }
>> 
>> +struct hvmemul_cache *hvmemul_cache_init(unsigned int nents)
>> +{
>> +    struct hvmemul_cache *cache = xmalloc_bytes(offsetof(struct
>> hvmemul_cache,
>> +                                                         ents[nents]));
>> +
>> +    if ( cache )
>> +    {
>> +        cache->num_ents = 0;
>> +        cache->max_ents = nents;
>> +    }
>> +
>> +    return cache;
>> +}
>> +
>>  bool hvmemul_read_cache(const struct hvmemul_cache *cache, paddr_t
>> gpa,
>>                          unsigned int level, void *buffer, unsigned int size)
>>  {
>> +    unsigned int i;
>> +
> 
> Here you could return false if cache is NULL...

This one could perhaps be considered, but ...

>> @@ -2674,6 +2721,35 @@ void hvmemul_write_cache(struct hvmemul_
>>                           unsigned int level, const void *buffer,
>>                           unsigned int size)
>>  {
>> +    unsigned int i;
>> +
> 
> ...and here just bail out. Thus making both functions safe to call with a 
> NULL cache.

... I'm pretty much opposed to this: The term "cache" might be slightly
confusing here, but I lack a better idea for a name. Its presence is
required for correctness. After all the series is not a performance
improvement, but a plain bug fix (generalizing what we were able to
special case for the actually observed problem in commit 91afb8139f
["x86/HVM: suppress I/O completion for port output"]). And with
that I'd rather leave the read side as is as well.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel