Re: [PATCH v2 2/3] x86/hvm: Allow writes to registers on the same page as MSI-X table

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.03.2023 14:52, Marek Marczykowski-Górecki wrote:
> On Tue, Mar 28, 2023 at 02:34:23PM +0200, Jan Beulich wrote:
>> On 28.03.2023 14:05, Marek Marczykowski-Górecki wrote:
>>> On Tue, Mar 28, 2023 at 01:28:44PM +0200, Roger Pau Monné wrote:
>>>> On Sat, Mar 25, 2023 at 03:49:23AM +0100, Marek Marczykowski-Górecki wrote:
>>>>> +static bool cf_check msixtbl_page_accept(
>>>>> +        const struct hvm_io_handler *handler, const ioreq_t *r)
>>>>> +{
>>>>> +    ASSERT(r->type == IOREQ_TYPE_COPY);
>>>>> +
>>>>> +    return msixtbl_page_handler_get_hwaddr(
>>>>> +            current->domain, r->addr, r->dir == IOREQ_WRITE);
>>>>
>>>> I think you want to accept it also if it's a write to the PBA, and
>>>> just drop it.  You should always pass write=false and then drop it in
>>>> msixtbl_page_write() if it falls in the PBA region (but still return
>>>> X86EMUL_OKAY).
>>>
>>> I don't want to interfere with msixtbl_mmio_page_ops, this handler is
>>> only about accesses not hitting actual MSI-X structures.
>>
>> In his functionally similar vPCI change I did ask Roger to handle the
>> "extra" space right from the same handlers. Maybe that's going to be
>> best here, too.
> 
> I have considered this option, but msixtbl_range() is already quite
> complex, adding yet another case there won't make it easier to follow.

Do you care about the case of msixtbl_addr_to_desc() returning NULL at
all for the purpose you have? Like in Roger's patch I'd assume
msixtbl_find_entry() needs extending what ranges it accepts; if need
be another parameter may be added to cover cases where the extended
coverage isn't wanted.

> I mean, technically I can probably merge those two handlers together,
> but I don't think it will result in nicer code. Especially since the
> general direction is to abandon split of MSI-X table access handling
> between Xen and QEMU and go with just QEMU doing it, hopefully at some
> point not needing msixtbl_mmio_ops anymore (but still needing the one
> for adjacent accesses).

Hmm, at this point I'm not convinced of this plan. Instead I was hoping
that once vPCI properly supports PVH DomU-s, we may also be able to make
use of it for HVM, delegating less to qemu rather than more.

Jan