* Ashish Kalra Ashish.Kalra@amd.com wrote:
if (addr <= ghcb && ghcb <= addr + size) {
/* Handle the case of a huge page containing the GHCB page */if (addr <= ghcb && ghcb < addr + size) { skipped_addr = true; break; }@@ -1131,9 +1132,8 @@ static void shutdown_all_aps(void) void snp_kexec_finish(void) { struct sev_es_runtime_data *data;
- unsigned long size, mask, ghcb; unsigned int level, cpu;
- unsigned long size;
- struct ghcb *ghcb;
So this patch just morphs the type of 'ghcb' from a typed pointer to unsigned long, while most 'ghcb' uses in coco/ are typed pointers?
That's just sloppy and fragile. Please just keep 'ghcb' a typed pointer, and introduce *another* variable for the virtual address to the hugepage.
pte_t *pte; if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP)) @@ -1157,11 +1157,14 @@ void snp_kexec_finish(void) for_each_possible_cpu(cpu) { data = per_cpu(runtime_data, cpu);
ghcb = &data->ghcb_page;pte = lookup_address((unsigned long)ghcb, &level);
ghcb = (unsigned long)&data->ghcb_page;
If 'ghcb' has the proper type then this ugly forced type-cast goes away.
pte = lookup_address(ghcb, &level);mask = page_level_mask(level);/* Handle the case of a huge page containing the GHCB page */ghcb &= mask;
This too calls for using a separate variable for this, because after this masking 'ghcb' is very much *not* the location of a GHCB page anymore...
set_pte_enc(pte, level, (void *)ghcb);
snp_set_memory_private((unsigned long)ghcb, (size / PAGE_SIZE));
snp_set_memory_private(ghcb, (size / PAGE_SIZE));
Do we know whether this is safe? Could the huge page around the GHCB page contain anything else? What is the structure of this memory area, is it all dedicated to the GHCB, or could it contain random other data?
Thanks,
Ingo