[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/7] x86/HVM: drop stdvga's "stdvga" struct member
On 10.09.2024 19:47, Andrew Cooper wrote: > On 10/09/2024 3:40 pm, Jan Beulich wrote: >> @@ -498,19 +483,17 @@ static bool cf_check stdvga_mem_accept( >> >> spin_lock(&s->lock); >> >> - if ( p->dir == IOREQ_WRITE && p->count > 1 ) >> + if ( p->dir != IOREQ_WRITE || p->count > 1 ) >> { >> /* >> * We cannot return X86EMUL_UNHANDLEABLE on anything other then the >> * first cycle of an I/O. So, since we cannot guarantee to always be >> * able to send buffered writes, we have to reject any multi-cycle >> - * I/O. >> + * I/O. And of course we have to reject all reads, for not being >> + * able to service them. >> */ >> goto reject; >> } >> - else if ( p->dir == IOREQ_READ && >> - (true || !s->stdvga) ) >> - goto reject; > > Before, we rejected on (WRITE && count) or READ, and I think we still do > afterwards given the boolean-ness of read/write. However, the comment > is distinctly less relevant. > > I think we now just end up with /* Only accept single writes. */ which > matches with with shuffling these into the bufioreq ring. I'd like to keep a little more, if you don't mind: /* * Only accept single writes, as that's the only thing we accelerate * using buffered ioreq handling. */ */ Not the least because writing this I noticed another flaw (or even two, depending on how one looks at it): ioreq_send_buffered() further refuses data_is_ptr requests. (Checking ->data_is_ptr is actually more important than ->count, as ->count will be unequal to 1 only if ->data_is_ptr is set, whereas ->count can also be 1 in that case.) Not the least because that "refusing" is actually returning "success" (0 == X86EMUL_OKAY == IOREQ_STATUS_HANDLED) on x86, and IO_ABORT on Arm. I.e. such requests would simply be lost on x86, and whether IO_ABORT is okay(ish) on Arm I simply don't know (I'll see if digging through history reveals intentions). And then - how can a buffered ioreq be a read, when there's nothing being returned? (I.e. I consider this an omission in what ioreq_send_buffered() refuses to process.) Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |