RE: [Patch v3 13/14] PCI: hv: Add hypercalls to read/write MMIO space

["Michael Kelley (LINUX)" <mikelley@microsoft.com>]

From: Wei Liu <wei.liu@kernel.org> Sent: Thursday, November 17, 2022 7:17 AM
>
> On Wed, Nov 16, 2022 at 10:41:36AM -0800, Michael Kelley wrote:
> [...]
> >
> > +static void hv_pci_read_mmio(struct device *dev, phys_addr_t gpa, int size, u32
> *val)
> > +{
> > +	struct hv_mmio_read_input *in;
> > +	struct hv_mmio_read_output *out;
> > +	u64 ret;
> > +
> > +	/*
> > +	 * Must be called with interrupts disabled so it is safe
> > +	 * to use the per-cpu input argument page.  Use it for
> > +	 * both input and output.
> > +	 */
> 
> Perhaps adding something along this line?
> 
> 	WARN_ON(!irqs_disabled());
> 
> I can fold this in if you agree.

These two new functions are only called within this module from code
that already has interrupts disabled (as added in Patch 14 of the series),
so I didn't do the extra check.  But I'm OK with adding it.  These functions
make a hypercall, so the additional check doesn't have enough perf
impact to matter.

Michael

> 
> > +	in = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > +	out = *this_cpu_ptr(hyperv_pcpu_input_arg) + sizeof(*in);
> > +	in->gpa = gpa;
> > +	in->size = size;
> > +
> > +	ret = hv_do_hypercall(HVCALL_MMIO_READ, in, out);
> > +	if (hv_result_success(ret)) {
> > +		switch (size) {
> > +		case 1:
> > +			*val = *(u8 *)(out->data);
> > +			break;
> > +		case 2:
> > +			*val = *(u16 *)(out->data);
> > +			break;
> > +		default:
> > +			*val = *(u32 *)(out->data);
> > +			break;
> > +		}
> > +	} else
> > +		dev_err(dev, "MMIO read hypercall error %llx addr %llx size %d\n",
> > +				ret, gpa, size);
> > +}
> > +
> > +static void hv_pci_write_mmio(struct device *dev, phys_addr_t gpa, int size, u32
> val)
> > +{
> > +	struct hv_mmio_write_input *in;
> > +	u64 ret;
> > +
> > +	/*
> > +	 * Must be called with interrupts disabled so it is safe
> > +	 * to use the per-cpu input argument memory.
> > +	 */
> 
> Ditto.
> 
> Thanks,
> Wei.