> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 03 March 2020 15:24
> To: Durrant, Paul <pdurr...@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Kevin Tian <kevin.t...@intel.com>; Wei
> Liu <w...@xen.org>; Paul
> Durrant <p...@xen.org>; Andrew Cooper <andrew.coop...@citrix.com>; Jun
> Nakajima
> <jun.nakaj...@intel.com>; Roger Pau Monné <roger....@citrix.com>
> Subject: RE: [EXTERNAL][PATCH v5 2/4] x86/HVM: implement memory read caching
> for insn emulation
>
> CAUTION: This email originated from outside of the organization. Do not click
> links or open
> attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On 03.03.2020 16:16, Durrant, Paul wrote:
> >> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of Jan
> >> Beulich
> >> Sent: 03 March 2020 10:17
> >>
> >> --- a/xen/arch/x86/hvm/emulate.c
> >> +++ b/xen/arch/x86/hvm/emulate.c
> >> @@ -28,6 +28,19 @@
> >> #include <asm/iocap.h>
> >> #include <asm/vm_event.h>
> >>
> >> +struct hvmemul_cache
> >> +{
> >> + /* The cache is disabled as long as num_ents > max_ents. */
> >> + unsigned int num_ents;
> >> + unsigned int max_ents;
> >> + struct {
> >> + paddr_t gpa:PADDR_BITS;
> >> + unsigned int :BITS_PER_LONG - PADDR_BITS - 8;
> >
> > Is clang ok with unnamed fields?
>
> Clang 5 at least is, and iirc this is a standard C feature.
>
Google didn't give me a conclusive answer which was I asked. I've not found
anything that says it's a gcc-ism though.
> >> +void hvmemul_cache_restore(struct vcpu *v, unsigned int token)
> >> +{
> >> + struct hvmemul_cache *cache = v->arch.hvm.hvm_io.cache;
> >> +
> >> + ASSERT(cache->num_ents > cache->max_ents);
> >> + cache->num_ents = token;
> >
> > ASSERT(token <= cache->max_ents) here?
>
> Hmm, not sure. Definitely not exactly as you say, as disabling
> in already disabled state would return max_ents + 1, and hence
> this value could also be fed back here.
Ok, I can see keeping that idempotent is useful.
> But even beyond that I
> don't see a need to restrict the value range here - anything
> larger than max_ents will simply result in the cache being
> disabled.
>
Fair enough.
Reviewed-by: Paul Durrant <pdurr...@amzn.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
