On 13.12.2022 23:26, Demi Marie Obenour wrote:
> This allows eliminating the former.
>
> Suggested-by: Andrew Cooper <andrew.coop...@citrix.com>
> Signed-off-by: Demi Marie Obenour <d...@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
with a couple of small remarks (some asking for minor adjustments):
> @@ -72,8 +72,8 @@ static uint8_t __read_mostly
> mtrr_epat_tbl[MTRR_NUM_TYPES][MEMORY_NUM_TYPES] =
> };
>
> /* Lookup table for PAT entry of a given PAT value in host PAT. */
> -static uint8_t __read_mostly pat_entry_tbl[PAT_TYPE_NUMS] =
> - { [0 ... PAT_TYPE_NUMS-1] = INVALID_MEM_TYPE };
> +static uint8_t __read_mostly pat_entry_tbl[X86_NUM_MT] =
> + { [0 ... X86_NUM_MT-1] = INVALID_MEM_TYPE };
When touching code like this, please also correct style (here: missing
blanks around '-').
> @@ -145,14 +145,14 @@ int hvm_vcpu_cacheattr_init(struct vcpu *v)
> m->mtrr_cap = (1u << 10) | (1u << 8) | num_var_ranges;
>
> v->arch.hvm.pat_cr =
> - ((uint64_t)PAT_TYPE_WRBACK) | /* PAT0: WB */
> - ((uint64_t)PAT_TYPE_WRTHROUGH << 8) | /* PAT1: WT */
> - ((uint64_t)PAT_TYPE_UC_MINUS << 16) | /* PAT2: UC- */
> - ((uint64_t)PAT_TYPE_UNCACHABLE << 24) | /* PAT3: UC */
> - ((uint64_t)PAT_TYPE_WRBACK << 32) | /* PAT4: WB */
> - ((uint64_t)PAT_TYPE_WRTHROUGH << 40) | /* PAT5: WT */
> - ((uint64_t)PAT_TYPE_UC_MINUS << 48) | /* PAT6: UC- */
> - ((uint64_t)PAT_TYPE_UNCACHABLE << 56); /* PAT7: UC */
> + ((uint64_t)X86_MT_WB) | /* PAT0: WB */
> + ((uint64_t)X86_MT_WT << 8) | /* PAT1: WT */
> + ((uint64_t)X86_MT_UCM << 16) | /* PAT2: UC- */
> + ((uint64_t)X86_MT_UC << 24) | /* PAT3: UC */
> + ((uint64_t)X86_MT_WB << 32) | /* PAT4: WB */
> + ((uint64_t)X86_MT_WT << 40) | /* PAT5: WT */
> + ((uint64_t)X86_MT_UCM << 48) | /* PAT6: UC- */
> + ((uint64_t)X86_MT_UC << 56); /* PAT7: UC */
As per my comment on the earlier patch the casts indeed want to stay, but
with how you had the earlier patch I wonder why you did keep them in this
version (and elsewhere below as well).
> @@ -356,7 +356,7 @@ uint32_t get_pat_flags(struct vcpu *v,
> */
> pat_entry_value = mtrr_epat_tbl[shadow_mtrr_type][guest_eff_mm_type];
> /* If conflit occurs(e.g host MTRR is UC, guest memory type is
> - * WB),set UC as effective memory. Here, returning PAT_TYPE_UNCACHABLE
> will
> + * WB),set UC as effective memory. Here, returning X86_MT_UC will
Would you mind at least adding the missing blank after the comma while
you touch the line?
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -573,7 +573,7 @@ int epte_get_entry_emt(struct domain *d, gfn_t gfn, mfn_t
> mfn,
> if ( gmtrr_mtype >= 0 )
> {
> *ipat = true;
> - return gmtrr_mtype != PAT_TYPE_UC_MINUS ? gmtrr_mtype
> + return gmtrr_mtype != X86_MT_UCM ? gmtrr_mtype
> : MTRR_TYPE_UNCACHABLE;
Please adjust indentation on this line then as well.
Jan
