Messages in this thread | | | Subject | Re: [PATCH Part1 v5 17/38] x86/mm: Add support to validate memory when changing C-bit | From | Brijesh Singh <> | Date | Wed, 25 Aug 2021 08:54:31 -0500 |
| |
On 8/25/21 6:06 AM, Borislav Petkov wrote: > > I really meant putting the beginning of that string at the very first > position on the line: > > if (WARN(hdr->end_entry > end_entry || cur_entry > hdr->cur_entry, > "SEV-SNP: PSC processing going backward, end_entry %d (got %d) cur_entry %d (got %d)\n", > end_entry, hdr->end_entry, cur_entry, hdr->cur_entry)) { > > Exactly like this! >
Noted.
> ... > >> +static void set_page_state(unsigned long vaddr, unsigned int npages, int op) >> +{ >> + unsigned long vaddr_end, next_vaddr; >> + struct snp_psc_desc *desc; >> + >> + vaddr = vaddr & PAGE_MASK; >> + vaddr_end = vaddr + (npages << PAGE_SHIFT); >> + >> + desc = kmalloc(sizeof(*desc), GFP_KERNEL_ACCOUNT); > > And again, from previous review: > > kzalloc() so that you don't have to memset() later in > __set_page_state(). >
I replied to your previous comment. Depending on the npages value, the __set_page_state() will be called multiple times and on each call it needs to clear desc before populate it. So, I do not see strong reason to use kzalloc() during the desc allocation. I thought you were okay with that explanation.
>> + if (!desc) >> + panic("SEV-SNP: failed to alloc memory for PSC descriptor\n"); > > "allocate" fits just fine too. >
Noted.
thanks
| |