Re: [Resend RFC PATCH V2 03/12] x86/Hyper-V: Add new hvcall guest address host visibility support

[Tianyu Lan]

Hi Christoph:
	Thanks for your review.

On 4/14/2021 11:40 PM, Christoph Hellwig wrote:
> > +/*
> > + * hv_set_mem_host_visibility - Set host visibility for specified memory.
> > + */
>
> I don't think this comment really clarifies anything over the function
> name.  What is 'host visibility'

OK. Will update the comment.

> > +int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility)
>
> Should size be a size_t?
> Should visibility be an enum of some kind?

Will update.

> > +int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility)
>
> Not sure what this does either.

Will add a comment.

> > +	local_irq_save(flags);
> > +	input_pcpu = (struct hv_input_modify_sparse_gpa_page_host_visibility **)
>
> Is there a chance we could find a shorter but still descriptive
> name for this variable?  Why do we need the cast?

Sure. The cast is to avoid build error due to "incompatible-pointer-types"
> > +#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
> > +#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
>
> pointlessly overlong line.