[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/7] x86/HVM: drop stdvga's "stdvga" struct member
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Tue, 10 Sep 2024 18:47:31 +0100
- Autocrypt: addr=andrew.cooper3@xxxxxxxxxx; keydata= xsFNBFLhNn8BEADVhE+Hb8i0GV6mihnnr/uiQQdPF8kUoFzCOPXkf7jQ5sLYeJa0cQi6Penp VtiFYznTairnVsN5J+ujSTIb+OlMSJUWV4opS7WVNnxHbFTPYZVQ3erv7NKc2iVizCRZ2Kxn srM1oPXWRic8BIAdYOKOloF2300SL/bIpeD+x7h3w9B/qez7nOin5NzkxgFoaUeIal12pXSR Q354FKFoy6Vh96gc4VRqte3jw8mPuJQpfws+Pb+swvSf/i1q1+1I4jsRQQh2m6OTADHIqg2E ofTYAEh7R5HfPx0EXoEDMdRjOeKn8+vvkAwhviWXTHlG3R1QkbE5M/oywnZ83udJmi+lxjJ5 YhQ5IzomvJ16H0Bq+TLyVLO/VRksp1VR9HxCzItLNCS8PdpYYz5TC204ViycobYU65WMpzWe LFAGn8jSS25XIpqv0Y9k87dLbctKKA14Ifw2kq5OIVu2FuX+3i446JOa2vpCI9GcjCzi3oHV e00bzYiHMIl0FICrNJU0Kjho8pdo0m2uxkn6SYEpogAy9pnatUlO+erL4LqFUO7GXSdBRbw5 gNt25XTLdSFuZtMxkY3tq8MFss5QnjhehCVPEpE6y9ZjI4XB8ad1G4oBHVGK5LMsvg22PfMJ ISWFSHoF/B5+lHkCKWkFxZ0gZn33ju5n6/FOdEx4B8cMJt+cWwARAQABzSlBbmRyZXcgQ29v cGVyIDxhbmRyZXcuY29vcGVyM0BjaXRyaXguY29tPsLBegQTAQgAJAIbAwULCQgHAwUVCgkI CwUWAgMBAAIeAQIXgAUCWKD95wIZAQAKCRBlw/kGpdefoHbdD/9AIoR3k6fKl+RFiFpyAhvO 59ttDFI7nIAnlYngev2XUR3acFElJATHSDO0ju+hqWqAb8kVijXLops0gOfqt3VPZq9cuHlh IMDquatGLzAadfFx2eQYIYT+FYuMoPZy/aTUazmJIDVxP7L383grjIkn+7tAv+qeDfE+txL4 SAm1UHNvmdfgL2/lcmL3xRh7sub3nJilM93RWX1Pe5LBSDXO45uzCGEdst6uSlzYR/MEr+5Z JQQ32JV64zwvf/aKaagSQSQMYNX9JFgfZ3TKWC1KJQbX5ssoX/5hNLqxMcZV3TN7kU8I3kjK mPec9+1nECOjjJSO/h4P0sBZyIUGfguwzhEeGf4sMCuSEM4xjCnwiBwftR17sr0spYcOpqET ZGcAmyYcNjy6CYadNCnfR40vhhWuCfNCBzWnUW0lFoo12wb0YnzoOLjvfD6OL3JjIUJNOmJy RCsJ5IA/Iz33RhSVRmROu+TztwuThClw63g7+hoyewv7BemKyuU6FTVhjjW+XUWmS/FzknSi dAG+insr0746cTPpSkGl3KAXeWDGJzve7/SBBfyznWCMGaf8E2P1oOdIZRxHgWj0zNr1+ooF /PzgLPiCI4OMUttTlEKChgbUTQ+5o0P080JojqfXwbPAyumbaYcQNiH1/xYbJdOFSiBv9rpt TQTBLzDKXok86M7BTQRS4TZ/ARAAkgqudHsp+hd82UVkvgnlqZjzz2vyrYfz7bkPtXaGb9H4 Rfo7mQsEQavEBdWWjbga6eMnDqtu+FC+qeTGYebToxEyp2lKDSoAsvt8w82tIlP/EbmRbDVn 7bhjBlfRcFjVYw8uVDPptT0TV47vpoCVkTwcyb6OltJrvg/QzV9f07DJswuda1JH3/qvYu0p vjPnYvCq4NsqY2XSdAJ02HrdYPFtNyPEntu1n1KK+gJrstjtw7KsZ4ygXYrsm/oCBiVW/OgU g/XIlGErkrxe4vQvJyVwg6YH653YTX5hLLUEL1NS4TCo47RP+wi6y+TnuAL36UtK/uFyEuPy wwrDVcC4cIFhYSfsO0BumEI65yu7a8aHbGfq2lW251UcoU48Z27ZUUZd2Dr6O/n8poQHbaTd 6bJJSjzGGHZVbRP9UQ3lkmkmc0+XCHmj5WhwNNYjgbbmML7y0fsJT5RgvefAIFfHBg7fTY/i kBEimoUsTEQz+N4hbKwo1hULfVxDJStE4sbPhjbsPCrlXf6W9CxSyQ0qmZ2bXsLQYRj2xqd1 bpA+1o1j2N4/au1R/uSiUFjewJdT/LX1EklKDcQwpk06Af/N7VZtSfEJeRV04unbsKVXWZAk uAJyDDKN99ziC0Wz5kcPyVD1HNf8bgaqGDzrv3TfYjwqayRFcMf7xJaL9xXedMcAEQEAAcLB XwQYAQgACQUCUuE2fwIbDAAKCRBlw/kGpdefoG4XEACD1Qf/er8EA7g23HMxYWd3FXHThrVQ HgiGdk5Yh632vjOm9L4sd/GCEACVQKjsu98e8o3ysitFlznEns5EAAXEbITrgKWXDDUWGYxd pnjj2u+GkVdsOAGk0kxczX6s+VRBhpbBI2PWnOsRJgU2n10PZ3mZD4Xu9kU2IXYmuW+e5KCA vTArRUdCrAtIa1k01sPipPPw6dfxx2e5asy21YOytzxuWFfJTGnVxZZSCyLUO83sh6OZhJkk b9rxL9wPmpN/t2IPaEKoAc0FTQZS36wAMOXkBh24PQ9gaLJvfPKpNzGD8XWR5HHF0NLIJhgg 4ZlEXQ2fVp3XrtocHqhu4UZR4koCijgB8sB7Tb0GCpwK+C4UePdFLfhKyRdSXuvY3AHJd4CP 4JzW0Bzq/WXY3XMOzUTYApGQpnUpdOmuQSfpV9MQO+/jo7r6yPbxT7CwRS5dcQPzUiuHLK9i nvjREdh84qycnx0/6dDroYhp0DFv4udxuAvt1h4wGwTPRQZerSm4xaYegEFusyhbZrI0U9tJ B8WrhBLXDiYlyJT6zOV2yZFuW47VrLsjYnHwn27hmxTC/7tvG3euCklmkn9Sl9IAKFu29RSo d5bD8kMSCYsTqtTfT6W4A3qHGvIDta3ptLYpIAOD2sY3GYq2nf3Bbzx81wZK14JdDDHUX2Rs 6+ahAA==
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Tue, 10 Sep 2024 17:47:40 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 10/09/2024 3:40 pm, Jan Beulich wrote:
> Two of its consumers are dead (in compile-time constant conditionals)
> and the only remaining ones are merely controlling (debugging) log
> messages.
Minor, but I'd phrase this as "merely controlling debug logging."
> Hence the field is now pointless to set, which in particular
> allows to get rid of the questionable conditional from which the field's
> value was established (afaict 551ceee97513 ["x86, hvm: stdvga cache
> always on"] had dropped too much of the earlier extra check that was
> there, and quite likely further checks were missing).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/hvm/stdvga.c
> +++ b/xen/arch/x86/hvm/stdvga.c
> @@ -103,7 +103,7 @@ static void vram_put(struct hvm_hw_stdvg
> static int stdvga_outb(uint64_t addr, uint8_t val)
> {
> struct hvm_hw_stdvga *s = ¤t->domain->arch.hvm.stdvga;
> - int rc = 1, prev_stdvga = s->stdvga;
> + int rc = 1;
This looks to also drop a MISRA essential-type complaint.
>
> switch ( addr )
> {
> @@ -132,19 +132,6 @@ static int stdvga_outb(uint64_t addr, ui
> break;
> }
>
> - /* When in standard vga mode, emulate here all writes to the vram buffer
> - * so we can immediately satisfy reads without waiting for qemu. */
> - s->stdvga = (s->sr[7] == 0x00);
> -
> - if ( !prev_stdvga && s->stdvga )
> - {
> - gdprintk(XENLOG_INFO, "entering stdvga mode\n");
> - }
> - else if ( prev_stdvga && !s->stdvga )
> - {
> - gdprintk(XENLOG_INFO, "leaving stdvga mode\n");
> - }
> -
> return rc;
> }
>
> @@ -425,7 +412,6 @@ static int cf_check stdvga_mem_write(
> const struct hvm_io_handler *handler, uint64_t addr, uint32_t size,
> uint64_t data)
> {
> - struct hvm_hw_stdvga *s = ¤t->domain->arch.hvm.stdvga;
> ioreq_t p = {
> .type = IOREQ_TYPE_COPY,
> .addr = addr,
> @@ -436,8 +422,7 @@ static int cf_check stdvga_mem_write(
> };
> struct ioreq_server *srv;
>
> - if ( true || !s->stdvga )
> - goto done;
> + goto done;
>
> /* Intercept mmio write */
> switch ( size )
> @@ -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.
~Andrew
|