Messages in this thread | | | Date | Wed, 28 Feb 2024 12:50:45 +0100 | From | Borislav Petkov <> | Subject | Re: [PATCH v8 03/16] virt: sev-guest: Add SNP guest request structure |
| |
On Thu, Feb 15, 2024 at 05:01:15PM +0530, Nikunj A Dadhania wrote: > +enum aead_algo {
Looks unused.
Add new struct definitions, etc, together with their first user - not preemptively.
Please audit all your patches.
> + SNP_AEAD_INVALID, > + SNP_AEAD_AES_256_GCM, > +}; > + > +struct snp_guest_msg_hdr { > + u8 authtag[MAX_AUTHTAG_LEN]; > + u64 msg_seqno; > + u8 rsvd1[8]; > + u8 algo; > + u8 hdr_version; > + u16 hdr_sz; > + u8 msg_type; > + u8 msg_version; > + u16 msg_sz; > + u32 rsvd2; > + u8 msg_vmpck; > + u8 rsvd3[35]; > +} __packed; > + > +struct snp_guest_msg { > + struct snp_guest_msg_hdr hdr; > + u8 payload[4000];
What Tom said.
> +} __packed; > + > +struct snp_guest_req { > + void *req_buf; > + size_t req_sz; > + > + void *resp_buf; > + size_t resp_sz; > + > + void *data; > + size_t data_npages; > + > + u64 exit_code; > + unsigned int vmpck_id; > + u8 msg_version; > + u8 msg_type; > +}; > + > #ifdef CONFIG_AMD_MEM_ENCRYPT > extern void __sev_es_ist_enter(struct pt_regs *regs); > extern void __sev_es_ist_exit(void); > @@ -223,7 +288,8 @@ void snp_set_memory_private(unsigned long vaddr, unsigned long npages); > void snp_set_wakeup_secondary_cpu(void); > bool snp_init(struct boot_params *bp); > void __init __noreturn snp_abort(void); > -int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, struct snp_guest_request_ioctl *rio); > +int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_data *input, > + struct snp_guest_request_ioctl *rio);
Much nicer!
..
> @@ -868,7 +890,6 @@ static int __init sev_guest_probe(struct platform_device *pdev) > /* initial the input address for guest request */
That comment reads wrong - might fix it while here.
> snp_dev->input.req_gpa = __pa(snp_dev->request); > snp_dev->input.resp_gpa = __pa(snp_dev->response); > - snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
> > ret = tsm_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type); > if (ret)
Rest looks ok. I'd chose shorter local vars if I were you but yours are ok too.
Thx.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |