On 25/06/2025 17:33, Vishal Annapurve wrote: > On Wed, Jun 18, 2025 at 7:58 AM Dave Hansen <dave.hansen@xxxxxxxxx> wrote: >> >> On 6/18/25 05:08, Adrian Hunter wrote: >>> --- a/arch/x86/kvm/vmx/tdx.c >>> +++ b/arch/x86/kvm/vmx/tdx.c >>> @@ -282,10 +282,10 @@ static void tdx_clear_page(struct page *page) >>> void *dest = page_to_virt(page); >>> unsigned long i; >>> >>> - /* >>> - * The page could have been poisoned. MOVDIR64B also clears >>> - * the poison bit so the kernel can safely use the page again. >>> - */ >>> + /* Machine check handler may have poisoned the page */ >>> + if (PageHWPoison(page)) >>> + return; > > IIUC, even if movdir64b stores contents on hwpoisoned pages, it's not > going to cause any trouble. No. PageHWPoison(page) means the page should not be touched. It must be freed back to the allocator where it will never be allocated again. > > This check should be (unlikely(PageHWPoison(page)) and even better 'unlikely' would be fine > probably should be omitted altogether if there are no side effects of > direct store to hwpoisoned pages. > >> >> I think the old comment needs to stay in some form. >> >> There are two kinds of poisons here: One from an integrity mismatch and >> the other because the hardware decided the memory is bad. MOVDIR64B >> clears the integrity one, but not the hardware one obviously. > > To ensure I understand correctly, Am I correct in saying: movdir64b > clearing the integrity poison is just hardware clearing the poison > bit, software will still treat that page as poisoned? Typically an integrity violation would have caused a machine check and the machine check handler would have marked the page SetPageHWPoison(page). So we really end up with only 2 cases: 1. page is fine and PageHWPoison(page) is false 2. page may have had an integrity violation or a hardware error (we can't tell which), and PageHWPoison(page) is true
