From: Jan Beulich <jbeulich@suse.com>
To: Oleksii <oleksii.kurochko@gmail.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>,
"Ross Lagerwall" <ross.lagerwall@citrix.com>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Julien Grall" <julien@xen.org>,
"Bertrand Marquis" <bertrand.marquis@arm.com>,
"Michal Orzel" <michal.orzel@amd.com>,
"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"George Dunlap" <george.dunlap@citrix.com>,
"Shawn Anastasio" <sanastasio@raptorengineering.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v8 02/17] xen: introduce generic non-atomic test_*bit()
Date: Mon, 6 May 2024 10:24:12 +0200 [thread overview]
Message-ID: <7f2eb0fb-3ed1-4248-90bd-2bc3551a97f1@suse.com> (raw)
In-Reply-To: <2d81e4700075b55f1885a4b1c7ee44ad046b35f2.camel@gmail.com>
On 06.05.2024 10:16, Oleksii wrote:
> On Mon, 2024-05-06 at 08:33 +0200, Jan Beulich wrote:
>> On 03.05.2024 19:15, Oleksii wrote:
>>> On Thu, 2024-04-25 at 17:35 +0200, Jan Beulich wrote:
>>>>> #include <asm/bitops.h>
>>>>>
>>>>> +#ifndef arch_check_bitop_size
>>>>> +#define arch_check_bitop_size(addr)
>>>>
>>>> Can this really do nothing? Passing the address of an object
>>>> smaller
>>>> than
>>>> bitop_uint_t will read past the object in the generic__*_bit()
>>>> functions.
>>> It seems RISC-V isn' happy with the following generic definition:
>>> extern void __bitop_bad_size(void);
>>>
>>> /* --------------------- Please tidy above here ----------------
>>> ----
>>> - */
>>>
>>> #include <asm/bitops.h>
>>>
>>> #ifndef arch_check_bitop_size
>>>
>>> #define bitop_bad_size(addr) sizeof(*(addr)) <
>>> sizeof(bitop_uint_t)
>>>
>>> #define arch_check_bitop_size(addr) \
>>> if ( bitop_bad_size(addr) ) __bitop_bad_size();
>>>
>>> #endif /* arch_check_bitop_size */
>>
>> I'm afraid you've re-discovered something that was also found during
>> the
>> original Arm porting effort. As nice and logical as it would (seem
>> to) be
>> to have bitop_uint_t match machine word size, there are places ...
>>
>>> The following errors occurs. bitop_uint_t for RISC-V is defined as
>>> unsigned long for now:
>>
>> ... where we assume such operations can be done on 32-bit quantities.
> Based on RISC-V spec machine word is 32-bit, so then I can just drop
> re-definition of bitop_uint_t in riscv/asm/types.h and use the
> definition of bitop_uint_t in xen/types.h.
> Also it will be needed to update __AMO() macros in <riscv>/asm/bitops.h
> in the following way:
> #if BITOP_BITS_PER_WORD == 64
> #define __AMO(op) "amo" #op ".d"
> #elif BITOP_BITS_PER_WORD == 32
> #define __AMO(op) "amo" #op ".w"
> #else
> #error "Unexpected BITS_PER_LONG"
> #endif
> Note: BITS_PER_LONG was changed to BITOP_BITS_PER_WORD !
>
> Only one question remains for me. Given that there are some operations whichcan be performed on 32-bit quantities, it seems to me that bitop_uint_t
> can only be uint32_t.
> Am I correct? If yes, do we need to have ability to redefine
> bitop_uint_t and BITOP_BITS_PER_WORD in xen/types.h:
> #ifndef BITOP_TYPE
> #define BITOP_BITS_PER_WORD 32
>
> typedef uint32_t bitop_uint_t;
> #endif
Probably not, right now. Hence me having said "As nice and logical as it
would (seem to) be" in the earlier reply.
Jan
next prev parent reply other threads:[~2024-05-06 8:24 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 10:04 [PATCH v8 00/17] Enable build of full Xen for RISC-V Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 01/17] xen/riscv: disable unnecessary configs Oleksii Kurochko
2024-04-18 7:14 ` Jan Beulich
2024-04-18 13:18 ` Oleksii
2024-04-17 10:04 ` [PATCH v8 02/17] xen: introduce generic non-atomic test_*bit() Oleksii Kurochko
2024-04-25 15:35 ` Jan Beulich
2024-04-26 8:14 ` Oleksii
2024-04-26 10:48 ` Jan Beulich
2024-05-03 17:15 ` Oleksii
2024-05-06 6:33 ` Jan Beulich
2024-05-06 8:16 ` Oleksii
2024-05-06 8:24 ` Jan Beulich [this message]
2024-04-17 10:04 ` [PATCH v8 03/17] xen/bitops: implement fls{l}() in common logic Oleksii Kurochko
2024-04-25 15:44 ` Jan Beulich
2024-04-26 8:21 ` Oleksii
2024-04-26 10:51 ` Jan Beulich
2024-04-26 12:09 ` Oleksii
2024-04-26 12:32 ` Jan Beulich
2024-04-17 10:04 ` [PATCH v8 04/17] xen/bitops: put __ffs() into linux compatible header Oleksii Kurochko
2024-04-25 15:47 ` Jan Beulich
2024-04-17 10:04 ` [PATCH v8 05/17] xen/riscv: introduce bitops.h Oleksii Kurochko
2024-04-25 15:51 ` Jan Beulich
2024-04-17 10:04 ` [PATCH v8 06/17] xen/riscv: introduce cmpxchg.h Oleksii Kurochko
2024-04-29 13:38 ` Jan Beulich
2024-04-17 10:04 ` [PATCH v8 07/17] xen/riscv: introduce io.h Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 08/17] xen/riscv: introduce atomic.h Oleksii Kurochko
2024-04-29 13:45 ` Jan Beulich
2024-05-02 8:33 ` Oleksii
2024-04-17 10:04 ` [PATCH v8 09/17] xen/riscv: introduce monitor.h Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 10/17] xen/riscv: add definition of __read_mostly Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 11/17] xen/riscv: add required things to current.h Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 12/17] xen/riscv: add minimal stuff to page.h to build full Xen Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 13/17] xen/riscv: add minimal stuff to mm.h " Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 14/17] xen/riscv: introduce vm_event_*() functions Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 15/17] xen/riscv: add minimal amount of stubs to build full Xen Oleksii Kurochko
2024-04-17 10:04 ` [PATCH v8 16/17] xen/riscv: enable full Xen build Oleksii Kurochko
2024-04-17 10:05 ` [PATCH v8 17/17] xen/README: add compiler and binutils versions for RISC-V64 Oleksii Kurochko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f2eb0fb-3ed1-4248-90bd-2bc3551a97f1@suse.com \
--to=jbeulich@suse.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=andrew.cooper3@citrix.com \
--cc=bertrand.marquis@arm.com \
--cc=george.dunlap@citrix.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=oleksii.kurochko@gmail.com \
--cc=roger.pau@citrix.com \
--cc=ross.lagerwall@citrix.com \
--cc=sanastasio@raptorengineering.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).